Skip to contentSkip to conversation

drivers/scsi/aic7xxx/aic7xxx_osm_pci.c - remove pointer comparison to 0

PATCH In review Joe Perches 3 participants 5 messages Feb 11, 2008

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 drivers/scsi/aic7xxx/aic7xxx_osm_pci.c - remove pointer comparison to 0

drivers/scsi/aic7xxx/aic7xxx_osm_pci.c +2 -2
index dd6e21d..65e194f 100644
@@ -301,7 +301,7 @@ ahc_linux_pci_reserve_io_region(struct ahc_softc *ahc, u_long *base)
 	*base = pci_resource_start(ahc->dev_softc, 0);
 	if (*base == 0)
 		return (ENOMEM);
-	if (request_region(*base, 256, "aic7xxx") == 0)
+	if (!request_region(*base, 256, "aic7xxx"))
 		return (ENOMEM);
 	return (0);
 }
@@ -318,7 +318,7 @@ ahc_linux_pci_reserve_mem_region(struct ahc_softc *ahc,
 	start = pci_resource_start(ahc->dev_softc, 1);
 	if (start != 0) {
 		*bus_addr = start;
-		if (request_mem_region(start, 0x1000, "aic7xxx") == 0)
+		if (!request_mem_region(start, 0x1000, "aic7xxx"))
 			error = ENOMEM;
 		if (error == 0) {
 			*maddr = ioremap_nocache(start, 256);
maintainer
#1 lore raw

drivers/scsi/aic7xxx/aic7xxx_osm_pci.c ahc_linux_pci_reserve_io_region(struct ahc_softc *ahc, u_long *base) · line 304

Quoted 2 lines · Joe Perches
drivers/scsi/aic7xxx/aic7xxx_osm_pci.c +1 -1
-       if (request_region(*base, 256, "aic7xxx") == 0)
+       if (!request_region(*base, 256, "aic7xxx"))

This patch is completely pointless. if (x == 0) and if (!x) mean identical things and there's no style standard preferring one form over another. There's a marginal preference for if (!x) over if (x == NULL) for pointers, but it's still up to a driver writer if they wish to use the marginally unpreferred form.

James

#2 lore raw
Quoted 5 lines · James Bottomley

drivers/scsi/aic7xxx/aic7xxx_osm_pci.c ahc_linux_pci_reserve_io_region(struct ahc_softc *ahc, u_long *base) · line 304

Quoted 2 lines · Joe Perches
drivers/scsi/aic7xxx/aic7xxx_osm_pci.c +1 -1
-       if (request_region(*base, 256, "aic7xxx") == 0)
+       if (!request_region(*base, 256, "aic7xxx"))

This patch is completely pointless.

It removes a sparse warning.

Quoted 3 lines

There's a marginal preference for if (!x) over if (x == NULL) for pointers, but it's still up to a driver writer if they wish to use the marginally unpreferred form.

Use == NULL then if you care to.

cheers, Joe

#3 lore raw
Quoted 8 lines · On Mon, 11 Feb 2008 09:08:05 -0800 Joe Perches wrote:
Quoted 5 lines · James Bottomley

drivers/scsi/aic7xxx/aic7xxx_osm_pci.c ahc_linux_pci_reserve_io_region(struct ahc_softc *ahc, u_long *base) · line 304

Quoted 2 lines · Joe Perches
drivers/scsi/aic7xxx/aic7xxx_osm_pci.c +1 -1
-       if (request_region(*base, 256, "aic7xxx") == 0)
+       if (!request_region(*base, 256, "aic7xxx"))

This patch is completely pointless.

It removes a sparse warning.

I try to say that in the patch description. Andrew also tries to enforce such errors/warnings in patch descriptions....

Quoted 5 lines
Quoted 3 lines

There's a marginal preference for if (!x) over if (x == NULL) for pointers, but it's still up to a driver writer if they wish to use the marginally unpreferred form.

Use == NULL then if you care to.

---
~Randy

maintainer
#4 lore raw
Quoted 7 lines · Randy Dunlap
Quoted 5 lines · On Mon, 11 Feb 2008 09:08:05 -0800 Joe Perches wrote:
Quoted 4 lines · James Bottomley
Quoted 2 lines · Joe Perches
drivers/scsi/aic7xxx/aic7xxx_osm_pci.c +1 -1
-       if (request_region(*base, 256, "aic7xxx") == 0)
+       if (!request_region(*base, 256, "aic7xxx"))
Quoted 6 lines
Quoted 3 lines
Quoted 1 line

This patch is completely pointless.

It removes a sparse warning.

I try to say that in the patch description. Andrew also tries to enforce such errors/warnings in patch descriptions....

Well, the aic7xxx subdirectory is a nightmare of CodingStyle non conformities ... you can see the return (ENOMEM) just in this patch. That's two problems: the brackets and non negative error returns which are later converted to negative ones thus inviting sign problems. The driver is also about 3x bigger than it should be because of the vestiges of the BSD glue layer. However, I think my life is too short to apply the 32,554 patches it would take to correct this an issue at a time.

This is one of those drivers we tolerate because we must and we fix up around the regions we have to touch.

James