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

[Satyam Sharma <satyam@infradead.org>]

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@infradead.org>
> > 
> > ---
> > 
> >  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.