fix corruption in qla1280

fix corruption in qla1280

[Arjan van de Ven]

Hi,

In C, doing >> 32 on a 32 bit wide variable is undefined behavior, and the
code that gets generated by gcc (using the cpu shr) effectively means that
it's a nop. The patch below to qla1280.c fixes such a case, doing
>>16 twice looks more expensive but gcc optimizes it out while keeping
correct behavior.

Greetings,
   Arjan van de Ven

--- linux-2.6.0-test1/drivers/scsi/qla1280.c~	2003-07-14 11:21:51.233657418 +0200
+++ linux-2.6.0-test1/drivers/scsi/qla1280.c	2003-07-14 11:21:51.234657286 +0200
@@ -327,7 +327,7 @@
 /* 3.16 */
 #ifdef QLA_64BIT_PTR
 #define pci_dma_lo32(a)		(a & 0xffffffff)
-#define pci_dma_hi32(a)		(a >> 32)
+#define pci_dma_hi32(a)		((a >> 16)>>16)
 #else
 #define pci_dma_lo32(a)		(a & 0xffffffff)
 #define pci_dma_hi32(a)		0

Re: fix corruption in qla1280

[Kurt Garloff]

[-- Attachment #1: Type: text/plain, Size: 789 bytes --]

Arjan,

On Mon, Jul 14, 2003 at 09:24:37AM +0000, Arjan van de Ven wrote:
> In C, doing >> 32 on a 32 bit wide variable is undefined behavior, and the
> code that gets generated by gcc (using the cpu shr) effectively means that
> it's a nop. The patch below to qla1280.c fixes such a case, doing
> >>16 twice looks more expensive but gcc optimizes it out while keeping
> correct behavior.

If it's really a 32 bit variable, 
#define pci_dma_hi32(a)		(0)
would be correct. Shifting a 32bit value 32times to the right is just 0.
What do I miss?

Regards,
-- 
Kurt Garloff  <garloff@suse.de>                          Eindhoven, NL
GPG key: See mail header, key servers                 SuSE Labs (Head)
SuSE Linux AG, Nuernberg, DE                            SCSI, Security

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: fix corruption in qla1280

[Arjan van de Ven]

On Mon, Jul 14, 2003 at 10:43:15PM +0200, Kurt Garloff wrote:
> Arjan,
> 
> On Mon, Jul 14, 2003 at 09:24:37AM +0000, Arjan van de Ven wrote:
> > In C, doing >> 32 on a 32 bit wide variable is undefined behavior, and the
> > code that gets generated by gcc (using the cpu shr) effectively means that
> > it's a nop. The patch below to qla1280.c fixes such a case, doing
> > >>16 twice looks more expensive but gcc optimizes it out while keeping
> > correct behavior.
> 
> If it's really a 32 bit variable, 
> #define pci_dma_hi32(a)		(0)
> would be correct. Shifting a 32bit value 32times to the right is just 0.
> What do I miss?

it's only a 32 bit variable on 32 bit platforms
(or rather, when dma_addr_t is 32 bit.. the ifdefs for that get evil)