Hello Alex, On 11/20/20 4:03 PM, Alejandro Colomar (man-pages) wrote: > Hi Omar, > > I found a wording of mine to be a bit confusing. > Please see below. > > Thanks, > > Alex > > On 11/20/20 3:06 PM, Alejandro Colomar (man-pages) wrote: >> Hi Omar and Michael, >> >> please, see below. >> >> Thanks, >> >> Alex >> >> On 11/20/20 12:29 AM, Alejandro Colomar (mailing lists; readonly) wrote: >>> Hi Omar, >>> >>> Please, see some fixes below: >>> >>> Michael, I've also some questions for you below >>> (you can grep for mtk to find those). >>> >>> Thanks, >>> >>> Alex >>> >>> On 11/18/20 8:18 PM, Omar Sandoval wrote: >>>> From: Omar Sandoval <osandov@xxxxxx> >>>> >>>> This adds a new page, encoded_io(7), providing an overview of encoded >>>> I/O and updates fcntl(2), open(2), and preadv2(2)/pwritev2(2) to >>>> reference it. >>>> >>>> Cc: Michael Kerrisk <mtk.manpages@xxxxxxxxx> >>>> Cc: linux-man <linux-man@xxxxxxxxxxxxxxx> >>>> Signed-off-by: Omar Sandoval <osandov@xxxxxx> >>>> --- >>>> This feature is not yet upstream. >>>> >>>> man2/fcntl.2 | 10 +- >>>> man2/open.2 | 23 +++ >>>> man2/readv.2 | 70 +++++++++ >>>> man7/encoded_io.7 | 369 ++++++++++++++++++++++++++++++++++++++++++++++ >>>> 4 files changed, 471 insertions(+), 1 deletion(-) >>>> create mode 100644 man7/encoded_io.7 >>>> >>>> diff --git a/man2/fcntl.2 b/man2/fcntl.2 >>>> index 546016617..b0d7fa2c3 100644 >>>> --- a/man2/fcntl.2 >>>> +++ b/man2/fcntl.2 >>>> @@ -221,8 +221,9 @@ On Linux, this command can change only the [...] >>>> +.PP >>>> +This may be extended in the future, so >>>> +.I iov[0].iov_len >>>> +must be set to >>>> +.I "sizeof(struct\ encoded_iov)" >>>> +for forward/backward compatibility. >>>> +The remaining buffers contain the encoded data. >>>> +.PP >>>> +.I compression >>>> +and >>>> +.I encryption >>>> +are the encoding fields. >>>> +.I compression >>>> +is >>>> +.B ENCODED_IOV_COMPRESSION_NONE >>>> +(zero) >>>> +or a filesystem-specific >>>> +.B ENCODED_IOV_COMPRESSION >>> >>> Maybe s/ENCODED_IOV_COMPRESSION/ENCODED_IOV_COMPRESSION_*/ >> >> Or s/ENCODED_IOV_COMPRESSION/ENCODED_IOV_COMPRESSION_/ >> >> I'm not sure about existing practice. >> >> Michael (mtk), what would you do here? I think I've tended towards the former (ENCODED_IOV_COMPRESSION_*) in the past. >> >>> >>>> +constant; >>>> +see >>>> +.BR Filesystem\ support . >>> >>> Please, write it as [.BR "Filesystem support" .] >>> >>> and maybe I would change it, to be more specific, to the following: >>> >>> [ >>> see >>> .B Filesystem support >>> below. >>> ] >>> >>> So that the reader clearly understands it's on the same page. >>> >>>> +.I encryption >>>> +is currently always >>>> +.B ENCODED_IOV_ENCRYPTION_NONE >>>> +(zero). >>>> +.PP >>>> +.I unencoded_len >>>> +is the length of the unencoded (i.e., decrypted and decompressed) data. >>>> +.I unencoded_offset >>>> +is the offset into the unencoded data where the data in the file begins >>> >>> The above wording is a bit unclear to me. >>> >>> I suggest the following: >>> >>> [ >>> .I unencoded_offset >>> is the offset from the begining of the file >>> to the first byte of the unencoded data >>> ] > > Now I've read it again, and my wording was even worse than yours. > I think yours can be understood after a few reads. > > However, I'll still try to reword mine to see if I add some value: > > [ > .I unencoded_offset > is the offset from the first byte of the unencoded data > to the first byte of logical data. > ] > > If you prefer yours, or a mix, that's fine. > >>> >>>> +(less than or equal to >>>> +.IR unencoded_len ). >>>> +.I len >>>> +is the length of the data in the file >>>> +(less than or equal to >>>> +.I unencoded_len >>>> +- >>> >>> Here's a question for Michael (mtk): >>> >>> I've seen (many) cases where these math operations >>> are written without spaces, >>> and in the same line (e.g., [.IR a + b]). >>> >>> I'd like to know your preferences on this, >>> or what is actually more extended in the manual pages, >>> to stick with only one of them. I suspect there's a lot of inconsistency across pages. For simple cases like this, I think writing it without spaces is fine, and perhaps even preferable. >>> >>>> +.IR unencoded_offset ). >>>> +See >>>> +.B Extent layout >>>> +below for some examples. >>>> +.I >>> >>> Were you maybe going to add something there? >>> >>> If not, please remove that [.I]. >>> >>>> +.PP >>>> +If the unencoded data is actually longer than >>>> +.IR unencoded_len , >>>> +then it is truncated; >>>> +if it is shorter, then it is extended with zeroes. >>>> +.PP >>>> + >>> >>> Please, remove that blank line. >>> >>>> +.BR pwritev2 () >>> >>> Should be [.BR pwritev2 (2)] >>> >>> Michael (mtk), >>> >>> Am I right in that? Please, confirm. Yes. References to functions documented in other pages should include the section number in parentheses. [...] >>>> +.PP >>>> +However, suppose we read 50 bytes into a file >>>> +which contains a single compressed extent. >>>> +The filesystem must still return the entire compressed extent >>>> +for us to be able to decompress it, >>>> +so >>>> +.I unencoded_len >>>> +would be the length of the entire decompressed extent. >>>> +However, because the read was at offset 50, >>>> +the first 50 bytes should be ignored. >>>> +Therefore, >>>> +.I unencoded_offset >>>> +would be 50, >>>> +and >>>> +.I len >>>> +would accordingly be >>>> +.IR unencoded_len\ -\ 50 . >>> >>> This formats everything as I, except for the last dot. >>> Replace by: >>> >>> [ >>> .I unencoded >>> - 50. >>> ] >>> >>> Michael (mtk), same as above: >>> to space, or not to space? That is the question :p In this case, perhaps .IR unencoded \-1 [...] >>>> +.SS Security >>>> +Encoded I/O creates the potential for some security issues: >>>> +.IP * 3 >>>> +Encoded writes allow writing arbitrary data which the kernel will decode on >>>> +a subsequent read. Decompression algorithms are complex and may have bugs >>>> +which can be exploited by maliciously crafted data. >>>> +.IP * >>>> +Encoded reads may return data which is not logically present in the file >>>> +(see the discussion of >>>> +.I len >>>> +vs. >>> >>> Please, s/vs./vs/ >>> See the reasons below: >>> >>> Michael (mtk), >>> >>> Here the renderer outputs a double space >>> (as for separating two sentences). >>> >>> Are you okay with that? Yes, that should probably be avoided. I'm not sure what the correct way is to prevent that in groff though. I mean, one could write .RI "vs.\ " unencoded_len but I think that simply creates a nonbreaking space, which is not exactly what is desired. [....] Thanks, Michael -- Michael Kerrisk Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/ Linux/UNIX System Programming Training: http://man7.org/training/
