On Thu, Jan 28, 2021 at 12:31 PM Michael Kerrisk (man-pages)
[off-list ref] wrote:

Hello Suren,

On 1/28/21 7:40 PM, Suren Baghdasaryan wrote:

quoted

On Thu, Jan 28, 2021 at 4:24 AM Michael Kerrisk (man-pages)
[off-list ref] wrote:

quoted

Hello Suren,

Thank you for writing this page! Some comments below.

Thanks for the review!
Couple questions below and I'll respin the new version once they are clarified.

Okay. See below.

quoted

quoted

On Wed, 20 Jan 2021 at 21:36, Suren Baghdasaryan [off-list ref] wrote:

quoted

[...]

Thanks for all the acks. That let's me know that you saw what I said.

quoted

quoted

quoted

RETURN VALUE
    On success, process_madvise() returns the number of bytes advised. This
    return value may be less than the total number of requested bytes, if an
    error occurred. The caller should check return value to determine whether
    a partial advice occurred.

So there are three return values possible,

Ok, I think I see your point. How about this instead:

Well, I'm glad you saw it, because I forgot to finish it. But yes,
you understood what I forgot to say.

quoted

RETURN VALUE
     On success, process_madvise() returns the number of bytes advised. This
     return value may be less than the total number of requested bytes, if an
     error occurred after some iovec elements were already processed. The caller
     should check the return value to determine whether a partial
advice occurred.

    On error, -1 is returned and errno is set appropriately.

We recently standardized some wording here:
s/appropriately/to indicate the error/.

quoted

quoted

quoted

+.PP
+The pointer
+.I iovec
+points to an array of iovec structures, defined in

"iovec" should be formatted as

.I iovec

I think it is formatted that way above. What am I missing?

But also in "an array of iovec structures"...

quoted

BTW, where should I be using .I vs .IR? I was looking for an answer
but could not find it.

.B / .I == bold/italic this line
.BR / .IR == alternate bold/italic with normal (Roman) font.

So:
.I iovec
.I iovec ,       # so that comma is not italic
.BR process_madvise ()
etc.

[...]

quoted

quoted

quoted

+.I iovec
+if one of its elements points to an invalid memory
+region in the remote process. No further elements will be
+processed beyond that point.
+.PP
+Permission to provide a hint to external process is governed by a
+ptrace access mode
+.B PTRACE_MODE_READ_REALCREDS
+check; see
+.BR ptrace (2)
+and
+.B CAP_SYS_ADMIN
+capability that caller should have in order to affect performance
+of an external process.

The preceding sentence is garbled. Missing words?

Maybe I worded it incorrectly. What I need to say here is that the
caller should have both PTRACE_MODE_READ_REALCREDS credentials and
CAP_SYS_ADMIN capability. The first part I shamelessly copy/pasted
from https://man7.org/linux/man-pages/man2/process_vm_readv.2.html and
tried adding the second one to it, obviously unsuccessfully. Any
advice on how to fix that?

I think you already got pretty close. How about:

[[
Permission to provide a hint to another process is governed by a
ptrace access mode
.B PTRACE_MODE_READ_REALCREDS
check (see
BR ptrace (2));
in addition, the caller must have the
.B CAP_SYS_ADMIN
capability.

In V2 I explanded a bit this part to explain why CAP_SYS_ADMIN is
needed. There were questions about that during my patch review which
adds this requirement
(https://lore.kernel.org/patchwork/patch/1363605), so I thought a
short explanation would be useful.

]]

[...]

quoted

quoted

quoted

+.TP
+.B ESRCH
+No process with ID
+.I pidfd
+exists.

Should this maybe be:
[[
The target process does not exist (i.e., it has terminated and
been waited on).
]]

See pidfd_send_signal(2).

I "borrowed" mine from
https://man7.org/linux/man-pages/man2/process_vm_readv.2.html but
either one sounds good to me. Maybe for pidfd_send_signal the wording
about termination is more important. Anyway, it's up to you. Just let
me know which one to use.

I think the pidfd_send_signal(2) wording fits better.

[...]

Thanks,

Michael

--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/