Re: [PATCH -mm] IPS SCSI driver: Check return of scsi_add_host()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On Sun, 2 Sep 2007, Jeff Garzik wrote:

> Satyam Sharma wrote:
> > drivers/scsi/ips.c: In function ‘ips_register_scsi’:
> > drivers/scsi/ips.c:6869:
> > warning: ignoring return value of ‘scsi_add_host’, declared with attribute
> > warn_unused_result
> > 
> > scsi_add_host() is __must_check, so let's check it's return and cleanup
> > appropriately on errors.
> > 
> > Signed-off-by: Satyam Sharma <satyam@xxxxxxxxxxxxx>
> > 
> > ---
> > 
> >  drivers/scsi/ips.c |    7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> > 
> > --- linux-2.6.23-rc4-mm1/drivers/scsi/ips.c~fix	2007-09-02
> > 20:21:27.000000000 +0530
> > +++ linux-2.6.23-rc4-mm1/drivers/scsi/ips.c	2007-09-02 20:25:58.000000000
> > +0530
> > @@ -6866,7 +6866,12 @@ ips_register_scsi(int index)
> >  	sh->max_channel = ha->nbus - 1;
> >  	sh->can_queue = ha->max_cmds - 1;
> >  -	scsi_add_host(sh, NULL);
> > +	if (scsi_add_host(sh, NULL)) {
> > +		IPS_PRINTK(KERN_WARNING, ha->pcidev, "Unable to add SCSI
> > host\n");
> > +		free_irq(ha->irq, ha);
> > +		scsi_host_put(sh);
> > +		return -1;
> 
> ACK, as long as you add a comment to this code and the request_irq() failure
> code:  the oldha/old irq is not restored upon failure, in this function.
> 
> That is a pre-existing bug, and not your fault, but your patch "continues in
> the same buggy tradition" :)  We should at least note the FIXME at each error
> handling code branch.

True, I did notice that (as well as the coding style issue you pointed
out) when making the patch, but felt lazy to actually set it right fully.
And that (when it's done) should be a separate patch anyway.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]
  Powered by Linux