[PATCH] drivers: scsi: aic7xxx: Fix positive error codes.

[Jason Wilkes]

Note:
There are more instances of the problem described below, but I
thought I'd explain the first one in detail, to make sure it's
worth fixing the others (and to make sure I didn't do anything
stupid, which I may have. I'm new to this :-). Alright, here we go!

A few drivers seem to return positive error codes internally, in
order to signal something to a static function in the same driver.
(see, for example, drivers/staging/lustre/lnet/lnet/lib-move.c)
That's unusual, but seems okay as long as they're consistent between
return codes and return code checks. However, a problem might arise
if +ERRORCODE rather than -ERRORCODE is returned to other layers
of the kernel (e.g., to the driver core). Let's do some exploring...

Here's a function that returns +ENOMEM
FILE: drivers/scsi/aic7xxx/aic7770_osm.c
FUNC: aic7770_map_registers

int
aic7770_map_registers(struct ahc_softc *ahc, u_int port)
{
... (clipped for brevity) ..

	if (!request_region(port, AHC_EISA_IOSIZE, "aic7xxx"))
		return (ENOMEM);

... (clipped for brevity) ..
}

This may not be a problem, unless we're passing the value
outside the current module. So who calls this function?
$ grep -r -C20 aic7770_map_registers

Looks like it's only ever called in:
FILE: drivers/scsi/aic7xxx/aic7770.c
FUNC: aic7770_config

int aic7770_config(struct ahc_softc *ahc, struct aic7770_identity *entry, u_int io)
{
... (code and stuff) ...

	error = aic7770_map_registers(ahc, io);
	if (error != 0)
		return (error);

... (code and stuff) ...
}

Hmm... Let's see who calls this guy.
$ grep -r -C20 aic7770_config

This too is only called once, in:
FILE: drivers/scsi/aic7xxx/aic7770_osm.c
FUNC: aic7770_probe

static int
aic7770_probe(struct device *dev)
{
... (blah blah blah) ...

	error = aic7770_config(ahc, aic7770_ident_table + edev->id.driver_data,
			       eisaBase);
	if (error != 0) {
		ahc->bsh.ioport = 0;
		ahc_free(ahc);
		return (error);
	}

... (blah blah blah) ...
}

Same deal as before. Okay, so who calls aic7770_probe?

Well, as expected, no one calls it, at least not by name.
It's just the .probe function in the following struct:

FILE: drivers/scsi/aic7xxx/aic7770_osm.c

static struct eisa_driver aic7770_driver = {
	.id_table	= aic7770_ids,
	.driver = {
		.name   = "aic7xxx",
		.probe  = aic7770_probe,
		.remove = aic7770_remove,
	}
};

So the return code *is* being passed to another layer of the kernel,
in which case it should probably be negative.

There are lots of similar examples in the same driver. I'd be happy
to fix them up, but I thought I should send this patch first, to make
sure I'm not doing anything obviously wrong.

Ooh! One more thing, just to be safe. Above, I assumed that grepping
the kernel tree would give us all the places where an identifier shows
up, but maybe this driver does something clever with the preprocessor.

$ grep -r '##' drivers/scsi/aic7xxx/ || cowsay hooray
 ________
< hooray >
 --------
        \   ^__^
         \  (oo)\_______
            (__)\       )\/\
                ||----w |
                ||     ||

Okay good, so there shouldn't be any ## magic messing with our greps.
Alright, I guess I should send this patch off now. Thanks for reading,
and apologies in advance if I'm a moron :-)

Signed-off-by: Jason Wilkes <letshaveanadventure@xxxxxxxxx>
---
 drivers/scsi/aic7xxx/aic7770.c     | 2 +-
 drivers/scsi/aic7xxx/aic7770_osm.c | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/aic7xxx/aic7770.c b/drivers/scsi/aic7xxx/aic7770.c
index 5000bd6..cecdea9 100644
--- a/drivers/scsi/aic7xxx/aic7770.c
+++ b/drivers/scsi/aic7xxx/aic7770.c
@@ -171,7 +171,7 @@ aic7770_config(struct ahc_softc *ahc, struct aic7770_identity *entry, u_int io)
 		break;
 	default:
 		printk("aic7770_config: invalid irq setting %d\n", intdef);
-		return (ENXIO);
+		return -ENXIO;
 	}
 
 	if ((intdef & EDGE_TRIG) != 0)
diff --git a/drivers/scsi/aic7xxx/aic7770_osm.c b/drivers/scsi/aic7xxx/aic7770_osm.c
index 3d401d0..50f030d 100644
--- a/drivers/scsi/aic7xxx/aic7770_osm.c
+++ b/drivers/scsi/aic7xxx/aic7770_osm.c
@@ -51,7 +51,7 @@ aic7770_map_registers(struct ahc_softc *ahc, u_int port)
 	 * Lock out other contenders for our i/o space.
 	 */
 	if (!request_region(port, AHC_EISA_IOSIZE, "aic7xxx"))
-		return (ENOMEM);
+		return -ENOMEM;
 	ahc->tag = BUS_SPACE_PIO;
 	ahc->bsh.ioport = port;
 	return (0);
@@ -87,10 +87,10 @@ aic7770_probe(struct device *dev)
 	sprintf(buf, "ahc_eisa:%d", eisaBase >> 12);
 	name = kstrdup(buf, GFP_ATOMIC);
 	if (name == NULL)
-		return (ENOMEM);
+		return -ENOMEM;
 	ahc = ahc_alloc(&aic7xxx_driver_template, name);
 	if (ahc == NULL)
-		return (ENOMEM);
+		return -ENOMEM;
 	error = aic7770_config(ahc, aic7770_ident_table + edev->id.driver_data,
 			       eisaBase);
 	if (error != 0) {
-- 
2.1.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/