Avoid a divide by zero by

1) Ensuring that both sd_scsi.c and sd_atapi.c report a device as
offline when its disksize cannot be determined.

2) Ensuring blksize is always non-zero, with a default value of 512.

3) Eliminating a couple of unneeded 'sectors' variables, which makes
the code easier for me to read if nothing else.

The bug was introduced when sd_atapi.c was changed to allow
'incomplete' implementations to be reported as online. REALLY
incomplete implementations (i.e. not even the disksize can be
determined) would be reported online with a blksize of zero.

Also fix a couple of knf nits in Mickey's last commit, and add another
check for an rpm of 0.

ok (pre-Mickey diffs) tdeval@ marc@ frantzen@

2 files changed, 41 insertions, 38 deletions

diff --git a/sys/scsi/sd_atapi.c b/sys/scsi/sd_atapi.c
index af2eac71f16..46d399f07dc 100644
--- a/sys/scsi/sd_atapi.c
+++ b/sys/scsi/sd_atapi.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: sd_atapi.c,v 1.3 2003/05/13 00:53:32 krw Exp $	*/
+/*	$OpenBSD: sd_atapi.c,v 1.4 2003/06/25 02:18:35 krw Exp $	*/
 /*	$NetBSD: sd_atapi.c,v 1.3 1998/08/31 22:28:07 cgd Exp $	*/
 
 /*
@@ -75,6 +75,7 @@ sd_atapibus_get_parms(sd, dp, flags)
 	struct atapi_capacity_descriptor *descp;
 	struct atapi_sd_mode_data sense_data;
 	char capacity_data[ATAPI_CAP_DESC_SIZE(1)];
+	u_int16_t rpm;
 	int error;
 
 	bzero(&scsi_cmd, sizeof scsi_cmd);
@@ -112,35 +113,37 @@ sd_atapibus_get_parms(sd, dp, flags)
 		break;
 	}
 
-	dp->disksize = _4btol(descp->nblks);
-	dp->blksize = _3btol(descp->blklen);
-
-	/*
-	 * First, set up standard fictitious geometry, a la sd_scsi.c.
-	 */
-	dp->heads = 64;
-	dp->sectors = 32;
-	dp->cyls = dp->disksize / (64 * 32);
-	dp->rot_rate = 3600;
-
 	/*
-	 * Then try to get something better.  If we can't, that's
-	 * still OK.
+	 * Try to determine disk size, block size and geometry.  If disk size
+	 * (in sectors) can be determined, then fake the rest if necessary.
 	 *
 	 * XXX Rigid geometry page?
 	 */
+	dp->rot_rate = 3600;
+	dp->blksize = _3btol(descp->blklen);
+	if (dp->blksize == 0)
+		dp->blksize = 512;
+	dp->disksize = _4btol(descp->nblks);
+	if (dp->disksize == 0)
+		return (SDGP_RESULT_OFFLINE);
+
 	error = atapi_mode_sense(sd->sc_link, ATAPI_FLEX_GEOMETRY_PAGE,
 	    (struct atapi_mode_header *)&sense_data, FLEXGEOMETRYPAGESIZE,
 	    flags, SDRETRIES, 20000);
 	SC_DEBUG(sd->sc_link, SDEV_DB2,
 	    ("sd_atapibus_get_parms: mode sense (flex) error=%d\n", error));
-	if (error != 0)
-		return (SDGP_RESULT_OK);
-
-	dp->heads = sense_data.pages.flex_geometry.nheads;
-	dp->sectors = sense_data.pages.flex_geometry.ph_sec_tr;
-	dp->cyls = _2btol(sense_data.pages.flex_geometry.ncyl);
-	dp->rot_rate = _2btol(sense_data.pages.flex_geometry.rot_rate);
+	if (error == 0) {
+		dp->heads = sense_data.pages.flex_geometry.nheads;
+		dp->sectors = sense_data.pages.flex_geometry.ph_sec_tr;
+		dp->cyls = _2btol(sense_data.pages.flex_geometry.ncyl);
+		rpm = _2btol(sense_data.pages.flex_geometry.rot_rate);
+		if (rpm)
+			dp->rot_rate = rpm;
+	} else {
+		dp->heads = 64;
+		dp->sectors = 32;
+		dp->cyls = dp->disksize / (64 * 32);
+	}
 	
 	return (SDGP_RESULT_OK);
 }
diff --git a/sys/scsi/sd_scsi.c b/sys/scsi/sd_scsi.c
index b43cbaac0f2..e6ea2d2e9dc 100644
--- a/sys/scsi/sd_scsi.c
+++ b/sys/scsi/sd_scsi.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: sd_scsi.c,v 1.5 2003/06/24 22:42:07 mickey Exp $	*/
+/*	$OpenBSD: sd_scsi.c,v 1.6 2003/06/25 02:18:35 krw Exp $	*/
 /*	$NetBSD: sd_scsi.c,v 1.8 1998/10/08 20:21:13 thorpej Exp $	*/
 
 /*-
@@ -126,11 +126,11 @@ sd_scsibus_get_optparms(sd, dp, flags)
 {
 	struct scsi_mode_sense scsi_cmd;
 	struct sd_scsibus_mode_sense_data scsi_sense;
-	u_long sectors;
 	int error;
 
 	dp->blksize = 512;
-	if ((sectors = scsi_size(sd->sc_link, flags)) == 0)
+	dp->disksize = scsi_size(sd->sc_link, flags);
+	if (dp->disksize == 0)
 		return (SDGP_RESULT_OFFLINE);		/* XXX? */
 
 	/* XXX
@@ -160,8 +160,7 @@ sd_scsibus_get_optparms(sd, dp, flags)
 	 */
 	dp->heads = 64;
 	dp->sectors = 32;
-	dp->cyls = sectors / (dp->heads * dp->sectors);
-	dp->disksize = sectors;
+	dp->cyls = dp->disksize / (dp->heads * dp->sectors);
 
 	return (SDGP_RESULT_OK);
 }
@@ -178,9 +177,8 @@ sd_scsibus_get_parms(sd, dp, flags)
 {
 	struct sd_scsibus_mode_sense_data scsi_sense;
 	union scsi_disk_pages *sense_pages;
-	u_long sectors;
-	int page, error;
 	u_int16_t rpm;
+	int page, error;
 
 	dp->rot_rate = 3600;
 
@@ -227,10 +225,12 @@ sd_scsibus_get_parms(sd, dp, flags)
 		if (dp->blksize == 0)
 			dp->blksize = 512;
 
-		sectors = scsi_size(sd->sc_link, flags);
-		dp->disksize = sectors;
-		sectors /= (dp->heads * dp->cyls);
-		dp->sectors = sectors;	/* XXX dubious on SCSI */
+		dp->disksize = scsi_size(sd->sc_link, flags);
+		if (dp->disksize == 0)
+			return (SDGP_RESULT_OFFLINE);
+
+		/* XXX dubious on SCSI */
+		dp->sectors = dp->disksize / (dp->heads * dp->cyls);
 
 		return (SDGP_RESULT_OK);
 	}
@@ -242,7 +242,7 @@ sd_scsibus_get_parms(sd, dp, flags)
 		     (size_t)scsi_sense.header.blk_desc_len);
 		dp->heads = sense_pages->flex_geometry.nheads;
 		dp->cyls = _2btol(sense_pages->flex_geometry.ncyl);
-		rpm  = _2btol(scsi_sense.pages.flex_geometry.rpm);
+		rpm = _2btol(scsi_sense.pages.flex_geometry.rpm);
 		if (rpm)
 			dp->rot_rate = rpm;
 		if (scsi_sense.header.blk_desc_len >= 8)
@@ -254,10 +254,8 @@ sd_scsibus_get_parms(sd, dp, flags)
 		dp->disksize = dp->heads * dp->cyls * dp->sectors;
 		if (dp->disksize == 0)
 			goto fake_it;
-
 		if (dp->blksize == 0)
 			dp->blksize = 512;
-
 		return (SDGP_RESULT_OK);
 	}
 
@@ -303,12 +301,14 @@ fake_it:
 	 * this depends on which controller (e.g. 1542C is
 	 * different. but we have to put SOMETHING here..)
 	 */
-	sectors = scsi_size(sd->sc_link, flags);
+	dp->disksize = scsi_size(sd->sc_link, flags);
 	dp->heads = 64;
 	dp->sectors = 32;
-	dp->cyls = sectors / (64 * 32);
+	dp->cyls = dp->disksize / (64 * 32);
 	dp->blksize = 512;
-	dp->disksize = sectors;
+
+	if (dp->disksize == 0)
+		return (SDGP_RESULT_OFFLINE);
 
 	return (SDGP_RESULT_OK);
 }