On Mon, Jul 4, 2011 at 7:31 PM, Kenneth R Westerback
<kwesterb...@rogers.com> wrote:
> On Mon, Jul 04, 2011 at 02:26:42PM -0700, andre...@zoho.com wrote:
>> cmd.c:
>>
>> x86* maxhead should be 255, not 256
>>
>> make geom less useless by using disklabel for cyl, if available
>
> geom is useless, period. Making it easier will only encourage people
> to use it.
given that fdisk becomes before disklabel in install.md, the geom will
be set, and mbr partitions made based on it, before disklabel is
invoked
>
>>
>> part.c:
>>
>> x86* maxhead should be 254, not 255 (other arches?)
>
> But 6 lines ago you say maxhead should be 255 not 256? Perhaps you can
> provide the valid ranges for all of these and your references for
> the claim. An example on how you manage to cause yourself a problem
> would also be helpful.
these are different maxheads. one is the maximum numbers and this is
the INDEXED maximum number, ie its 0 based. you should read the code
because the two functions are checking for different things; the
relationship between the offsets was the same as now. e.g., one checks
for cyl > 1024 and this function cyl > 1023. please read the original
before the patch
my reference is a bug in certain versions of msdos that don't allow
head > 254 (INDEXED) or 255 (TOTAL)
also, most oses, obsd included, generate their equivalent of fake
disklabel with sectors fixed at 63, heads at 255, and variable length
cyls
why allow heads to go to 256?
>
>>
>> maxcyl gets set to 1023 before this function gets called, so it's a
>> redundant check
>>
>> Index: src/sbin/fdisk/cmd.c
>> ===================================================================
>> RCS file: /cvs/src/sbin/fdisk/cmd.c,v
>> retrieving revision 1.45
>> diff -p -u -r1.45 cmd.c
>> --- src/sbin/fdisk/cmd.c B B B 2 Jul 2010 02:54:09 -0000 B B B 1.45
>> +++ src/sbin/fdisk/cmd.c B B B 4 Jul 2011 20:55:22 -0000
>> @@ -67,9 +67,10 @@ Xreinit(cmd_t *cmd, disk_t *disk, mbr_t
>> B int
>> B Xdisk(cmd_t *cmd, disk_t *disk, mbr_t *mbr, mbr_t *tt, int offset)
>> B {
>> - B B int maxcyl B = 1024;
>> - B B int maxhead = 256;
>> - B B int maxsec B = 63;
>> + B B u_int32_t maxcyl B = 1024;
>> + B B u_int32_t maxhead = 255;
>> + B B u_int32_t maxsec B = 63;
>> + B B u_int32_t dmaxcyl = 0;
>
> What's the point of u_int32_t?
if you look at the definition of struct defining disk->label's type, you'd
know
that applies to dmaxcyl only, changed others for consistency
>
>>
>> B B B /* Print out disk info */
>> B B B DISK_printmetrics(disk, cmd->args);
>> @@ -80,8 +81,15 @@ Xdisk(cmd_t *cmd, disk_t *disk, mbr_t *m
>> B B B maxsec B = 9999999;
>> B #endif
>>
>> + B B if (disk->label)
>> + B B B B B B dmaxcyl = disk->label->cylinders;
>> +
>> B B B /* Ask for new info */
>> B B B if (ask_yn("Change disk geometry?")) {
>> + B B B B B B if (dmaxcyl > maxcyl) {
>> + B B B B B B B B B B printf("Warning setting cyl beyond %u
forces LBA\n", maxcyl);
>> + B B B B B B B B B B maxcyl = dmaxcyl;
>> + B B B B B B }
>>
>
> Since ask_num won't permit numbers > max, how will LBA be forced?
sigh
changing geom doesn't write the disklabel so no, it doesn't force lba,
or anything for that matter.
lba will be forced if you partition beyond cyl 1023 (INDEXED). this is
confirmed by reading part.c ...
"forced" as in fdisk will populate the 4 bit lba word in the mbr
>
>> B B B B B B B disk->real->cylinders = ask_num("BIOS Cylinders",
ASK_DEC,
>> B B B B B B B B B disk->real->cylinders, 1, maxcyl, NULL);
>> B B B B B B B disk->real->heads = ask_num("BIOS Heads", ASK_DEC,
>> Index: src/sbin/fdisk/part.c
>> ===================================================================
>> RCS file: /cvs/src/sbin/fdisk/part.c,v
>> retrieving revision 1.50
>> diff -p -u -r1.50 part.c
>> --- src/sbin/fdisk/part.c B B 29 Apr 2009 22:58:24 -0000 B B B 1.50
>> +++ src/sbin/fdisk/part.c B B 4 Jul 2011 20:38:24 -0000
>> @@ -212,12 +212,10 @@ PRT_parse(disk_t *disk, void *prt, off_t
>> B int
>> B PRT_check_chs(prt_t *partn)
>> B {
>> - B B if ( (partn->shead > 255) ||
>> + B B if ( (partn->shead > 254) ||
>> B B B B B B B (partn->ssect >63) ||
>> - B B B B B B (partn->scyl > 1023) ||
>> - B B B B B B (partn->ehead >255) ||
>> - B B B B B B (partn->esect >63) ||
>> - B B B B B B (partn->ecyl > 1023) )
>> + B B B B B B (partn->ehead >254) ||
>> + B B B B B B (partn->esect >63) )
>> B B B {
>> B B B B B B B return 0;
>> B B B }
>>
>
> .... Ken
