Hi Brian, On 2/4/23 08:19, Brian Inglis wrote: [...]
reboot.2 LINUX_REBOOT_MAGIC* adding hex birth datesLGTM(arguably should remove the decimals, or all values, and cryptic comment from doc?),Not sure what you mean.See below - decimal (and added hex) "BCD" values of Linus and kid's birthdates!
[...]
I'd prefer here (\-32000Ki, +32000Ki). Decimals on multipliers induce doubts on how much precision you kept; round numbers make it clear.-the supplied value is clamped to the range (\-32768000, +32768000). +the supplied value is clamped to the range (\-31.25Mi, +31.25Mi).Dithered about representing 32kKi - again magnitude seemed more important to document, but did not consider using decimals might introduce uncertainty about precision!
Actually, I should have suggested 32'000Ki :p
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].-is outside the range [0..999,999,999]. +is outside the range [0..999\(aq999\(aq999].-field was not in the range 0 to 999999999 or +field was not in the range 0 to 999\(aq999\(aq999 orSame here: [0, 999'999'999]If we are changing to consistent interval notation in a separate patch, should we replace the closed inclusive limit strings of "[0, ...999]" by open exclusive limits e.g. "[0, 1G)" etc. or would semi-open be too ambiguous, as intervals seen in the sample so far are either open (...) or closed [...]? I presume doc readers have less familiarity with maths and computer arithmetic than engineers or technical devs may require.
For the general case, I think it's more readable to use closed intervals; they are more commonly used in the man-pages so far, so I guess people are more used to reading them. However, we might consider exceptions if they are reasonable for some cases. Regarding familiarity to programmers, I expect most of them would be familiar with semi-open intervals, and as Ingo reminded me recently, we use them everyday in C in the form of `for (int i = 0; i < n; i++)`, so I'm not averse to it just by that. But we see closed intervals more often, probably because it removes one subtraction in your head and so it's simpler to read.
When 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-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 toI don't think that notation is meaningful documentation to most readers. The original decimal value is easier to comprehend.
Agree, I just brainfarted.
Even the hex 0x2000000-1 would be a bit more informative (0x20Mi-1).An odd binary power does not bring a value quickly to mind, and has to be decoded to 2^5*2^20-1 to make any sense: I had to evaluate it to be sure! With large values, the magnitude is more important to get across clearly with binary or decimal suffixes, although the value must be exact.
Agreed. Let's try with 32Mi-1.
In these cases, where you added the hex equivalent, I think it would need a comma as a separator, and maybe some connector?-(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)That's the decimal and "BCD" values of Linus and kid's birthdates if you look at the hex, to which the following "...meaningful" comment refers. Suggest we separate with a dash, or I wondered if we should just drop the "cute" values and comments?
I'd rather drop the decimal values. They are more meaningful as hex.
They are in the header if anyone other than Linus cares.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.-this limit was 0x2000000 (32\ MB). +this limit was 0x200\(aq0000 (32\ MiB).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-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 (to do this though. It's probably a lot of work. I can do that after your patches.Agree here, that's why I wanted you to have a look at the changes first.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
