Subject: Minor bugs with AmigaDOS partition processing

From: [email-masked] (Michael L. Hitch)

There are a couple of minor bug in sd.c regarding the processing of the
partition blocks.  The first one is that the partition list is supposed to
be terminated with a 0xFFFFFFFF value, not a NULL value [it would be possible
to have a partition block at block 0, although I think most RDB editors put
the RDSK block there.]  Since a block number of 0xFFFFFFFF is out of range
for any partition, the read of that block will fail and terminate the
partition block processing - so the current code works like it's supposed to.
The second bug is in computing the end of the partition.  The UPPERCYL field
in the disk environment is the cylinder number of the last cylinder of the
parition.  The current code computes the last block of the partition as the
block the last cylinder begins on, so does not include the last cylinder of
the partition.  The only effect this bug has is that the partition size will
be one cylinder short.

  There is also a bug in swapgeneric.c when prompting for the root device.
A character pointer is used to get the unit number, but is never initialized.
This bug only shows up when running the newer kernels with the older loadbsd
that doesn't set up the boothowto flags.  I've modified my loadbsd to accept
another option to ask for the root device and this patch appears to allow
the root device to be specified.  [It still requires that the device specified
have both a BSDR and BSDS partition though.]

Michael

*** v644/sys/arch/amiga/dev/sd.c	Fri Sep  3 16:38:56 1993
--- sd.c	Fri Oct 15 08:32:17 1993
***************
*** 448,454 ****
  
  	/* ok, go thru the partition blocks, and check for BSD ID's */
  	
! 	for (npbnum = rdb->rdb_PartitionList; npbnum; )
  	  {
  	    u_int start_block, end_block, nblocks;
  	    u_int reserved;
--- 448,454 ----
  
  	/* ok, go thru the partition blocks, and check for BSD ID's */
  	
! 	for (npbnum = rdb->rdb_PartitionList; npbnum != 0xffffffff; )
  	  {
  	    u_int start_block, end_block, nblocks;
  	    u_int reserved;
***************
*** 466,472 ****
  	    npbnum      = pb->pb_Next;
  	    start_block = pb->pb_Environment[DE_LOWCYL] * dl->d_secpercyl;
  	    reserved    = pb->pb_Environment[DE_RESERVEDBLKS];
! 	    end_block   = pb->pb_Environment[DE_UPPERCYL] * dl->d_secpercyl;
  
  	    part = -1;
  	    switch (pb->pb_Environment[DE_DOSTYPE])
--- 466,472 ----
  	    npbnum      = pb->pb_Next;
  	    start_block = pb->pb_Environment[DE_LOWCYL] * dl->d_secpercyl;
  	    reserved    = pb->pb_Environment[DE_RESERVEDBLKS];
! 	    end_block   = (pb->pb_Environment[DE_UPPERCYL] + 1) * dl->d_secpercyl;
  
  	    part = -1;
  	    switch (pb->pb_Environment[DE_DOSTYPE])

*** v644/sys/arch/amiga/amiga/swapgeneric.c	Thu Jul 29 15:56:04 1993
--- swapgeneric.c	Sun Oct 17 13:58:36 1993
***************
*** 66,71 ****
--- 66,72 ----
  } genericconf[] = {
  /*	{ (caddr_t)&rddriver,	"rd",	makedev(2, 0),	},*/
  	{ (caddr_t)&sddriver,	"sd",	makedev(4, 0),	},
+ 	{ (caddr_t)&sddriver,	"rz",	makedev(5, 0),	},
  	{ 0 },
  };
  
***************
*** 103,108 ****
--- 104,110 ----
  		printf("\n");
  		goto retry;
  gotit:
+ 		cp = name + 1;
  		if (*++cp < '0' || *cp > '9') {
  			printf("bad/missing unit number\n");
  			goto retry;

-- 
Michael L. Hitch			INTERNET:  [email-masked]
Computer Consultant			BITNET:  [email-masked]
Office of Systems and Computing Services
Montana State University	Bozeman, MT	USA

---

From: [email-masked] (Michael L. Hitch)

On Oct 28, 12:13pm, William J. Coldwell wrote:
> From: [email-masked] (Michael L. Hitch)
> 
> >Since a block number of 0xFFFFFFFF is out of range
> Generally, the RDB can be put anywhere within the RDB_LIMIT, which I believe is  
> 25 blocks (off of the top of my head).  Someone must have gotten confused  
> between NULL and HB_NULL, which is the -1L as you have pointed out.  You can  
> have a partition at block 0, and it would be perfectly legal to do so.

  RDB_LIMIT is 16.  I should have said the current code works like it's
supposed as long as there is no PART block in block 0 (which is certainly
valid, but I'm not aware of any RDB editors that would do that).

> >parition.
> Since this is read-only, that doesn't seem to be so traumatic.  There is  
> another set of cases where older partition programs didn't set the uppercyl  
> field correctly, as in.. either being 0, or being -1, and a seperate case of  
> loaders sticking stuff up on the RDB, like device drivers which autoboot ROMs  
> load off.  In this case, there is very little need to parse the RDB all the way  
> out to the end of the uppercyl.  So IMHO, the best way is to ignore uppercyl,  
> and just read a block at a time, following the PART thread from the RDB.  I  
> came to this conclusion after thinking that it would be fine just do an  
> AllocMem() on the uppercyl-lowercyl values, then doing one read for the whole  
> RDB.  We want the BSD boot to be as quick as possible ;-).

  I think you mis-interpreted my comment.  The UPPERCYL value is for each
partition and defines the size of the partition.  The code in sd.c is computing
the size of each BSD partition based on the PART block for that partition and
does not compute it correctly.  I'm not certain, but I would guess that newfs
uses the partition size when it is creating the new file system.  The current
code would result in a partition that is short by one cylinder (number of
surfaces * number of blocks/track).  If you patch _sddebug to set it non-zero,
you will see the starting and ending blocks of each BSD partition as it is
processed.  The current code leaves a gap between each partition.

Michael

---

From: William J. Coldwell <[email-masked]>

>partition and defines the size of the partition.

Yep, I sure did.  I thought that you were talking about the RDB uppercyl.. I  
will now crawl back under my administrative rock ;-)

Bill