Messages in this thread Patch in this message | | | Date | Mon, 11 Aug 2003 18:13:50 -0700 | From | Jeff Woods <> | Subject | Re: [PATCH] oops in sd_shutdown |
| |
At +0200 12:28 AM 8/12/2003, Andries Brouwer wrote (in part): >The obvious patch (with whitespace damage) > >diff -u --recursive --new-file -X /linux/dontdiff a/drivers/scsi/sd.c >b/drivers/scsi/sd.c >--- a/drivers/scsi/sd.c Mon Jul 28 05:39:31 2003 >+++ b/drivers/scsi/sd.c Tue Aug 12 01:24:51 2003 >@@ -1351,10 +1351,14 @@ > static void sd_shutdown(struct device *dev) > { > struct scsi_device *sdp = to_scsi_device(dev); >- struct scsi_disk *sdkp = dev_get_drvdata(dev); >+ struct scsi_disk *sdkp; > struct scsi_request *sreq; > int retries, res; > >+ sdkp = dev_get_drvdata(dev); >+ if (!sdkp) >+ return; /* this can happen */ >+ > if (!sdp->online || !sdkp->WCE) > return;
Looking only at the above code snippet, I'd suggest something more like: ~~~~~~~~~~~~~~~~~~ diff -u --recursive --new-file -X /linux/dontdiff a/drivers/scsi/sd.c b/drivers/scsi/sd.c --- a/drivers/scsi/sd.c Mon Jul 28 05:39:31 2003 +++ b/drivers/scsi/sd.c Tue Aug 12 01:24:51 2003 @@ -1351,10 +1351,14 @@ static void sd_shutdown(struct device *dev) { struct scsi_device *sdp = to_scsi_device(dev); struct scsi_disk *sdkp = dev_get_drvdata(dev); struct scsi_request *sreq; int retries, res;
- if (!sdp->online || !sdkp->WCE) + if (!sdp || !sdp->online || !sdkp || !sdkp->WCE) return; ~~~~~~~~~~~~~~~~~~ If sdp can *never* be NULL *and* this is a performance-critical code-path then perhaps it makes sense to leave off the "!sdp || " which I added to the logic, but it seems a very small cost to pay for the insurance checking in sd_shutdown().
-- Jeff Woods <kazrak+kernel@cesmail.net>
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |