Skip to contentSkip to conversation

sanitize ->bios_param prototype

PATCH James Bottomley 3 participants 3 messages Oct 25, 2002 1 message not in the archive SCSI

lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

James Bottomley

author
#1 lore raw
Quoted 10 lines · [email protected] said:

(aka Scsi_Disk or Disk) to each and every lowlevel driver, although this structure should be privated to the sd driver.

All bios_param implementation do only use two fields: .device and .capacity. This patch passes down those two directly and gets rid of 99% of the sd.h inclusions (*).

I've tried to not break any driver with this patch, but given the number of compiler errors in the current tree I might have missed one or two.

I'm getting:

Loading scsi_mod module
/lib/scsi_mod.o: unresolved symbol __udivdi3

The problem seems to be the definition of sector_t for the division. If I make this fix:

===== drivers/scsi/scsicam.c 1.10 vs edited =====

drivers/scsi/scsicam.c Fri Oct 25 12:10:40 2002 renamed from drivers/scsi/scsicam.c Thu Oct 24 19:29:22 2002 +2 -2
@@ -80,11 +80,11 @@
        if (ret || ip[0] > 255 || ip[1] > 63) {
                ip[0] = 64;
                ip[1] = 32;
-               if ((capacity / (ip[0] * ip[1])) > 65534) {
+               if (((unsigned long)capacity / (ip[0] * ip[1])) > 65534) {
                        ip[0] = 255;
                        ip[1] = 63;
                }
-               ip[2] = capacity / (ip[0] * ip[1]);
+               ip[2] = (unsigned long)capacity / (ip[0] * ip[1]);
        }
 
        return 0;

Then everything works fine. This is obviously not the correct fix, but it's as good as the previous code, so it should be OK.

James

#2 lore raw
Quoted 12 lines · James Bottomley
Quoted 10 lines · [email protected] said:

(aka Scsi_Disk or Disk) to each and every lowlevel driver, although this structure should be privated to the sd driver.

All bios_param implementation do only use two fields: .device and .capacity. This patch passes down those two directly and gets rid of 99% of the sd.h inclusions (*).

I've tried to not break any driver with this patch, but given the number of compiler errors in the current tree I might have missed one or two.

Quoted 7 lines

I'm getting:

Loading scsi_mod module
/lib/scsi_mod.o: unresolved symbol __udivdi3

The problem seems to be the definition of sector_t for the division. If I make this fix:

Ooops - I tested without CONFIG_LBD defined where it's 32bit so I couldn't trap this bug. I think the right fix would be to use sector_div (warning, hand-edit patch):

drivers/scsi/scsicam.c Fri Oct 25 12:10:40 2002 renamed from drivers/scsi/scsicam.c Thu Oct 24 19:29:22 2002 +3 -2
@@ -80,11 +80,12 @@
        if (ret || ip[0] > 255 || ip[1] > 63) {
                ip[0] = 64;
                ip[1] = 32;
-               if ((capacity / (ip[0] * ip[1])) > 65534) {
+               if (((unsigned long)capacity / (ip[0] * ip[1])) > 65534) {
                        ip[0] = 255;
                        ip[1] = 63;
                }
-               ip[2] = capacity / (ip[0] * ip[1]);
+         	sector_div(capacity, (ip[0] * ip[1]));      
+		ip[2] = capacity;
        }
 
        return 0;
#3 lore raw
In reply to James Bottomley
Quoted 24 lines · James Bottomley

Loading scsi_mod module
/lib/scsi_mod.o: unresolved symbol __udivdi3

The problem seems to be the definition of sector_t for the division. If I make this fix:

===== drivers/scsi/scsicam.c 1.10 vs edited =====

drivers/scsi/scsicam.c Fri Oct 25 12:10:40 2002 renamed from drivers/scsi/scsicam.c Thu Oct 24 19:29:22 2002 +2 -2
@@ -80,11 +80,11 @@
        if (ret || ip[0] > 255 || ip[1] > 63) {
                ip[0] = 64;
                ip[1] = 32;
-               if ((capacity / (ip[0] * ip[1])) > 65534) {
+               if (((unsigned long)capacity / (ip[0] * ip[1])) > 65534) {
                        ip[0] = 255;
                        ip[1] = 63;
                }
-               ip[2] = capacity / (ip[0] * ip[1]);
+               ip[2] = (unsigned long)capacity / (ip[0] * ip[1]);
        }
 
        return 0;
Quoted 2 lines

Then everything works fine. This is obviously not the correct fix, but it's as good as the previous code, so it should be OK.

I didn't see hch's patch, but you can always write

	if (ret || ip[0] > 255 || ip[1] > 63) {
		ip[0] = 64;
		ip[1] = 32;
		if ((capacity >> 11) > 65534) {
			ip[0] = 255;
			ip[1] = 63;
		}
		if (capacity > 65535*63*255)
			ip[2] = 65535;
		else
			ip[2] = (unsigned long)capacity / (ip[0] * ip[1]);
	}

Andries

(ip[2] is returned in a short, so returning larger values is meaningless; moreover, the current specification says that ip[2] is ignored)