Re: [PATCH] process_vm_{read,write}v(3): initial man pages

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thursday 29 March 2012 14:42:49 Michael Kerrisk (man-pages) wrote:
> .SH SYNOPSIS
> .B #include <sys/uio.h>
> .nf
> .BI "ssize_t process_vm_readv(pid_t " pid ,

i think there should be a blank line between sys/uio.h and the prototypes

> These system calls transfer data between the address space
> of the calling process ("the local process") and the process identified by
> .IR pid
> ("the remote process").

you set up terms as if you're going to use them, but then later continue to 
say "the calling process" and "the process identified by pid".  wouldn't it be 
better to convert all of those to "local" and "remote" ?

> .IR remote_vec
> is a pointer to an array describing address ranges in the process
> .IR pid ,
> and
> .IR riovcnt
> specifies the number of items in

personally, i prefer "elements" over "items"

> is a pointer to an array describing address ranges in the calling process,
> and
> .IR liovcnt
> specifies the specifies the number of items in

got "specifies the" duplicated here

> The
> .BR process_vm_writev ()
> system call is the converse of

"inverse" might be better, but either works

> .BR process_vm_readv ()\(emit
> transfers data from the calling process to the process

i don't like how this renders.  there's no spacing between the func and the 
next word:
	process_vm_readv()—it

imo, there should be:
	process_vm_readv() — it

> or the addresses refer to regions that are inaccessible in the local
> process,

might be better phrased as:
	... inaccessible to the local process ...

> .\" FIXME: What does the following sentence mean?

heh, i was just about to suggest a clarification.  consider the case where you 
want to read a C string from a remote process.  you don't know its length, so 
it is probably cheaper to read too much data and then discard the rest than it 
is to read it byte by byte in a single syscall.

further consider the string lies in the last page of a valid mapping.  you 
don't want to always say "read 500 bytes" because if the first 200 bytes are 
the last 200 bytes of the last page, the remaining 300 bytes cover invalid 
memory, and so no data will be read.

instead, you'd split the remote read array into two elements and have them 
merge back into a single write array entry.  the first read entry goes up to 
the page boundary while the second starts on the next page boundary.

> Keep this in mind when attempting to
> extract data of unknown length (such as C strings which are
> null-terminated) by avoiding spanning memory pages (typically 4KiB).

... by avoiding spanning memory pages (typically 4KiB) in a single iovec 
element.

> Note, however, that these system calls do not check the memory regions
> in the remote process until just before doing the read/write.
> Consequently, a partial read/write may result if one of the
> .I remote_vec
> elements points to an invalid memory region in the remote process.
> No further reads/writes will be attempted beyond that point.

should clarify that the partial read/write applies to iovec elements 
granularity and not byte granularity.  i.e. the syscall won't return a partial 
read that splits a single iovect element.

> In order to read from or write to another process,
> either the caller must have the capability
> .BR CAP_SYS_PTRACE ,
> or
> the real, effective, and saved set user IDs
> of the target process must match the real user ID of the caller
> .I and
> the real, effective, and saved set group IDs
> of the target process must match the real group ID of the caller.

the "set" phrasing here seems off.  should it be:
	... the real, effective, and saved set of user IDs ...
i.e. add the word "of"

> .SH "RETURN VALUE"
> On success,
> .BR process_vm_readv ()
> returns the number of bytes read and
> .BR process_vm_writev ()
> returns the number of bytes written.
> (This return value may be less than the total number of requested bytes,
> if a partial read/write occurred.
> The caller should check the return value to determine whether
> a partial read/write occurred.)

the stuff in parenthesis is larger than not.  maybe just drop them ?

> .B ENOMEM
> Out of memory.

this can be a little confusing to people based on the statements earlier that 
the kernel doesn't do any copying.  perhaps clarify that the kernel allocates 
copies of the iovec structures/etc... for internal state and that can OOM ?
-mike

Attachment: signature.asc
Description: This is a digitally signed message part.


[Index of Archives]     [Kernel Documentation]     [Netdev]     [Linux Ethernet Bridging]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]

  Powered by Linux