On Fri, 15 Apr 2011 11:10:39 -0700, Randy Dunlap wrote:
>> diff --git a/Documentation/x86/boot.txt b/Documentation/x86/boot.txt
>> index 9b7221a..7696e20 100644
>> --- a/Documentation/x86/boot.txt
>> +++ b/Documentation/x86/boot.txt
>> -Protocol 2.07: (Kernel 2.6.24) Added paravirtualised boot protocol.
>> - Introduced hardware_subarch and hardware_subarch_data
>> - and KEEP_SEGMENTS flag in load_flags.
>> +Protocol 2.07: (Kernel 2.6.24) Added paravirtualized boot protocol,
>
> We accept UK or US spellings.
Normally I wouldn't bother changing such spellings one way or the other,
but I was already changing the line and US spellings appear to be dominant.
>> -01F4/4 2.04+(2 syssize The size of the 32-bit code in 16-byte paras
>> +01F4/4 2.04+(2 sys_size The size of the 32-bit code
>
>Why drop the "in 16-byte paragraphs" part?
Two reasons:
* The whole word "paragraphs" wouldn't fit nicely on the line.
* That detail is already explained in the full description of sys_size
(which is where such information belongs):
Field name: sys_size
Type: read
Offset/size: 0x1f4/4 (protocol 2.04+) 0x1f4/2 (protocol ALL)
Protocol: 2.04+
The size of the protected-mode code in units of 16-byte paragraphs.
For protocol versions older than 2.04, this field is only two bytes
wide and therefore cannot be trusted for the size of a kernel if
the LOAD_HIGH flag is set.
>> - If set to a nonzero value, contains a pointer to a NUL-terminated
>> + If set to a nonzero value, it contains a pointer to a NUL-terminated
>
> non-zero ?
Yes, that would be better; I'll send an additional patch to change all
ocurrences of `nonzero' to `non-zero'.
>> - relocatable kernel at a nonstandard address it will have to modify
>> + relocatable kernel at a nonstandard address, it must modify
>
> non-standard
Indeed.
>> - The 64-bit physical pointer to NULL terminated single linked list of
>> - struct setup_data. This is used to define a more extensible boot
>> - parameters passing mechanism. The definition of struct setup_data is
>> - as follow:
>> + A 64-bit physical pointer to point to a NULL-terminated singly-linked list of
>> + struct setup_data. This is used to define a more extensible mechanism for
>> + passing boot parameters. The definition of struct setup_data is as follows:
>
> Some places in the text use NUL-terminated while this one uses NULL-terminated.
> Would be good to be consistent. Oh, also just found "null-terminated".
Yes, I should have caught the `NUL'.
However, `NULL' and `null' are a bit more interesting; I discussed this with
Michael Kerrisk (who maintains man-pages, as I'm sure you know) in early 2010:
Subject: Re: [PATCH 9/9] "null terminated" -> "null-terminated"
Message-ID: <cfd18e0f1001161102r3ad26d50i6e846504a6c35995@xxxxxxxxxxxxxx>
http://marc.info/?l=linux-man&m=126366859117181&q=raw
...
On Sun, Jan 10, 2010 at 12:45 PM, Michael Witten <mfwitten@xxxxxxxxx> wrote:
> The preferred spelling seems to be "null-terminated".
It's "NULL-terminated" when talking of pointers (e.g., the NULL at the
end of an array of pointers), but "null-terminated" when talking of
the '\0' at the end of a string...
...
Interestingly, `man ascii' (from man-pages) lists the null character's
symbolic name as `NUL'; go figure... maybe I should lobby Kerrisk for:
s/null-terminated/NUL-terminated/g
:-P
I'll check things out again and send a follow-up patch.
>> +while in real mode (basically, it needs to be within the bottom mebibyte).
>
> mebibyte. ugh. oh well.
Yes, well, I wasn't a fan of those unit names either at first, but I like
the idea of using the SI units properly, and mebibyte (etc.) is standardized
by the ISO (apparently).
>> -of the low megabyte as possible.
>> +of the bottom mebibyte as possible.
>
> That changes seems unnecessary/unhelpful.
I specifically wanted to change `low' to `bottom' in order to keep the
terminology consistent; plus, I was already going to change `megabyte'
to `mebibyte'.
>> - unsigned long base_ptr; /* base address for real-mode segment */
>> + unsigned long base_ptr; /* base address for real-mode segment */
>
> changing tab to space. does not matter.
I disagree, because the tab in question is in an utterly bizarre place
(between `base_ptr;' and `/* base...'. Moreover, it really only makes
sense to use tabs in order to set the indentation level; other uses
are less stable, if you will.
>> - base_ptr = 0x90000; /* Relocated */
>> + base_ptr = 0x90000; /* Relocated */
>
>ditto
ditto :-)
>> -The 32-bit (non-real-mode) kernel starts at offset (setup_sects+1)*512
>> -in the kernel file (again, if setup_sects == 0 the real value is 4.)
>> -It should be loaded at address 0x10000 for Image/zImage kernels and
>> +The protected-mode kernel starts at offset "(setup_sects+1)*512"
>> +in the kernel file (again, if (setup_sects == 0) { setup_sects = 4; }).
>
> The C code isn't especially better than what it is replacing IMO.
Frankly I think the whole parenthetical statement should be removed; it's
already stated in 3 places (2 within the text, and once within the code).
How about this:
In the kernel file, the protected-mode code starts at offset
"(setup_sects+1)*512" (or 0xa00 if setup_sects is 0, because
setup_sects is treated as though it were 4); the code should
be loaded at address 0x10000 for Image/zImage kernels and at
0x100000 for bzImage kernels.
The right edge of the paragraph even lines up nicely! :-D
> Thanks.
Thanks for the comments!
Sincerely,
Michael Witten
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
