On Mon, Jul 04, 2011 at 07:59:41PM -0430, Andres Perera wrote:
> 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
Not sure what you are saying here. My point was that the use of CHS
geometry is useless. LBA is all that is used/tested these days. The
use of CHS should be hard and attempted only by those who know all
the rules already.
>
> >
> >>
> >> 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
I spent some time reviewing the code. I was asking only for a clear
statement of your analysis.
>
> my reference is a bug in certain versions of msdos that don't allow
> head > 254 (INDEXED) or 255 (TOTAL)
This is not a reference. This is just restating your opinion. A reference
would be a link, a cut and paste of a manual, etc.
>
> also, most oses, obsd included, generate their equivalent of fake
> disklabel with sectors fixed at 63, heads at 255, and variable length
> cyls
This is true. How is it relevant to the actual limits that those
fiddling with the complexitites of CHS should have available?
>
> why allow heads to go to 256?
Why not? OpenBSD is not MSDOS, certain version of which you think
have a bug.
>
> >
> >>
> >> 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 ?? ?? ??2 Jul 2010 02:54:09 -0000 ?? ?? ?? 1.45
> >> +++ src/sbin/fdisk/cmd.c ?? ?? ??4 Jul 2011 20:55:22 -0000
> >> @@ -67,9 +67,10 @@ Xreinit(cmd_t *cmd, disk_t *disk, mbr_t
> >> ??int
> >> ??Xdisk(cmd_t *cmd, disk_t *disk, mbr_t *mbr, mbr_t *tt, int offset)
> >> ??{
> >> - ?? ?? int maxcyl ??= 1024;
> >> - ?? ?? int maxhead = 256;
> >> - ?? ?? int maxsec ??= 63;
> >> + ?? ?? u_int32_t maxcyl ??= 1024;
> >> + ?? ?? u_int32_t maxhead = 255;
> >> + ?? ?? u_int32_t maxsec ??= 63;
> >> + ?? ?? 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
If you read the definition of ask_num() you'd know why I asked.
>
> that applies to dmaxcyl only, changed others for consistency
>
> >
> >>
> >> ?? ?? ?? /* Print out disk info */
> >> ?? ?? ?? DISK_printmetrics(disk, cmd->args);
> >> @@ -80,8 +81,15 @@ Xdisk(cmd_t *cmd, disk_t *disk, mbr_t *m
> >> ?? ?? ?? maxsec ??= 9999999;
> >> ??#endif
> >>
> >> + ?? ?? if (disk->label)
> >> + ?? ?? ?? ?? ?? ?? dmaxcyl = disk->label->cylinders;
> >> +
> >> ?? ?? ?? /* Ask for new info */
> >> ?? ?? ?? if (ask_yn("Change disk geometry?")) {
> >> + ?? ?? ?? ?? ?? ?? if (dmaxcyl > maxcyl) {
> >> + ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? printf("Warning setting cyl beyond %u
> >> forces LBA\n", maxcyl);
> >> + ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? maxcyl = dmaxcyl;
> >> + ?? ?? ?? ?? ?? ?? }
> >>
> >
> > 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.
Do you mean write the MBR?
>
> lba will be forced if you partition beyond cyl 1023 (INDEXED). this is
> confirmed by reading part.c ...
The point being that at this point (ask_num() with low bound of 1 and
high of 1024) you cannot configure >1024 cylinders. So the message
is confusing. People will, based on experience, conclude they should be
able to force LBA by entering a number >1024. And complain.
>
> "forced" as in fdisk will populate the 4 bit lba word in the mbr
4 bit lba word?
.... Ken
>
> >
> >> ?? ?? ?? ?? ?? ?? ?? disk->real->cylinders = ask_num("BIOS Cylinders",
> >> ASK_DEC,
> >> ?? ?? ?? ?? ?? ?? ?? ?? ?? disk->real->cylinders, 1, maxcyl, NULL);
> >> ?? ?? ?? ?? ?? ?? ?? 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 ?? ?? 29 Apr 2009 22:58:24 -0000 ?? ?? ??1.50
> >> +++ src/sbin/fdisk/part.c ?? ?? 4 Jul 2011 20:38:24 -0000
> >> @@ -212,12 +212,10 @@ PRT_parse(disk_t *disk, void *prt, off_t
> >> ??int
> >> ??PRT_check_chs(prt_t *partn)
> >> ??{
> >> - ?? ?? if ( (partn->shead > 255) ||
> >> + ?? ?? if ( (partn->shead > 254) ||
> >> ?? ?? ?? ?? ?? ?? ?? (partn->ssect >63) ||
> >> - ?? ?? ?? ?? ?? ?? (partn->scyl > 1023) ||
> >> - ?? ?? ?? ?? ?? ?? (partn->ehead >255) ||
> >> - ?? ?? ?? ?? ?? ?? (partn->esect >63) ||
> >> - ?? ?? ?? ?? ?? ?? (partn->ecyl > 1023) )
> >> + ?? ?? ?? ?? ?? ?? (partn->ehead >254) ||
> >> + ?? ?? ?? ?? ?? ?? (partn->esect >63) )
> >> ?? ?? ?? {
> >> ?? ?? ?? ?? ?? ?? ?? return 0;
> >> ?? ?? ?? }
> >>
> >
> > .... Ken
