On 2/2/23 23:29, Brian Inglis wrote:> Hi Alex, Hi Brian,
Took your views on board and changed man2 pages. Attached summary only has file names and changed lines.
I prefer inline in the email :)
Would like feedback on what to continue doing and what to forget doing before starting man3?
See below.
Of note for review are open.2 octal perms,
The octals read a bit weirder than the others. Please keep them in a separate patch, so we can decide on it later. But I wouldn't discard it for now.
reboot.2 LINUX_REBOOT_MAGIC* adding hex birth dates
LGTM
(arguably should remove the decimals, or all values, and cryptic comment from doc?),
Not sure what you mean.
statsf.2 hex *_MAGIC,
LGTM
changing large arbitrary values to SI/IEC suffix forms (as the exact decimal values are not as informative or useful),
But I'd use multipliers that result in exact values. See below.
and feature docs using yyyy\(aqmmL (no example *code* was changed, although comments were).
LGTM. --- -the supplied value is clamped to the range (\-32768000, +32768000). +the supplied value is clamped to the range (\-31.25Mi, +31.25Mi).I'd prefer here (\-32000Ki, +32000Ki). Decimals on multipliers induce doubts on how much precision you kept; round numbers make it clear.
-is outside the range [0..999,999,999]. +is outside the range [0..999\(aq999\(aq999].Please fix also the format of the range, now that you're at it (in a separate commit, if you don't mind). I prefer mathematical notation, where possible: [0, 999'999'999].
-field was not in the range 0 to 999999999 or +field was not in the range 0 to 999\(aq999\(aq999 or Same here: [0, 999'999'999] -source, a maximum of 33554431 bytes is returned by a single call to +source, a maximum of 32Mi-1 bytes is returned by a single call toWhen the value is not an exact one, as here where it's the multiplier minus one, I prefer a more correct mathematical notation: 2^25-1
-(that is, 0xfee1dead) and +(that is, 0xfee1\(aqdead) and -(that is, 672274793). +(that is, 672\(aq274\(aq793 0x2812\(aq1969). -(that is, 85072278) +(that is, 85\(aq072\(aq278 0x0512\(aq1996) -(that is, 369367448) +(that is, 369\(aq367\(aq448 0x1604\(aq1998) -(that is, 537993216) +(that is, 537\(aq993\(aq216 0x2011\(aq2000)In these cases, where you added the hex equivalent, I think it would need a comma as a separator, and maybe some connector?
-this limit was 0x2000000 (32\ MB). +this limit was 0x200\(aq0000 (32\ MiB).Could you please separate the bugfixes such as this one in a different patch, if you don't mind? I don't care about the order of them, though.
-AFS_SUPER_MAGIC 0x5346414f -ANON_INODE_FS_MAGIC 0x09041934 /* Anonymous inode FS (for +AFS_SUPER_MAGIC 0x5346\(aq414f +ANON_INODE_FS_MAGIC 0x0904\(aq1934 /* Anonymous inode FS (I'm getting a bit confusing while reading the diff. The \(aq syntax is definitely a bit confusing when mixed with other random characters that the brain doesn't recognize as words. We can solve this by using \[aq] notation, which I like more personally. Please use this syntax. We'll also need some global fixes to change the notation all across the pages. I'm not asking you to do this though. It's probably a lot of work. I can do that after your patches.
Other than those minor comments, I like the diff very much. Please continue :) Cheers, Alex -- <http://www.alejandro-colomar.es/> GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5
Attachment:
OpenPGP_signature
Description: OpenPGP digital signature