[PATCH 5/5 v2]scsi:hosts.c Fix warning: variable 'rval' set but not used

[PATCH 5/5 v2]scsi:hosts.c Fix warning: variable 'rval' set but not used

[Justin P. Mattock]

The below patch fixes a warning message generated by gcc 4.6.0
  CC      drivers/scsi/hosts.o
drivers/scsi/hosts.c: In function 'scsi_host_alloc':
drivers/scsi/hosts.c:328:6: warning: variable 'rval' set but not used
 Signed-off-by: James Bottomley <James.Bottomley@suse.de>
 Signed-off-by: Justin P. Mattock <justinmattock@gmail.com>

---
 drivers/scsi/hosts.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
index 6660fa9..4cc99b7 100644
--- a/drivers/scsi/hosts.c
+++ b/drivers/scsi/hosts.c
@@ -325,7 +325,6 @@ struct Scsi_Host *scsi_host_alloc(struct scsi_host_template *sht, int privsize)
 {
 	struct Scsi_Host *shost;
 	gfp_t gfp_mask = GFP_KERNEL;
-	int rval;
 
 	if (sht->unchecked_isa_dma && privsize)
 		gfp_mask |= __GFP_DMA;
@@ -420,7 +419,8 @@ struct Scsi_Host *scsi_host_alloc(struct scsi_host_template *sht, int privsize)
 	shost->ehandler = kthread_run(scsi_error_handler, shost,
 			"scsi_eh_%d", shost->host_no);
 	if (IS_ERR(shost->ehandler)) {
-		rval = PTR_ERR(shost->ehandler);
+		printk(KERN_WARNING "scsi%d: error handler thread failed to spawn, error = %ld\n", 
+			shost->host_no, PTR_ERR(shost->ehandler));
 		goto fail_kfree;
 	}
 
-- 
1.7.1.rc1.21.gf3bd6

Re: [PATCH 5/5 v2]scsi:hosts.c Fix warning: variable 'rval' set but not used

[James Bottomley]

On Fri, 2010-06-18 at 13:16 -0700, Justin P. Mattock wrote:
> The below patch fixes a warning message generated by gcc 4.6.0
>   CC      drivers/scsi/hosts.o
> drivers/scsi/hosts.c: In function 'scsi_host_alloc':
> drivers/scsi/hosts.c:328:6: warning: variable 'rval' set but not used

OK, patch looks fine now, thanks!  But this isn't:

>  Signed-off-by: James Bottomley <James.Bottomley@suse.de>

You need to read Documentation/SubmittingPatches for the full story, but
you can't just add signed-off-by; you need an explicit email from the
person saying that ... plus, signoffs should follow the natural
progression of the patch, so if it's your patch, and you send it to me,
I'll add my signoff before I put it in the tree.  This has a legal basis
in the DCO (see SubmittinPatches).  We're a lot looser on the various
other tags, though.

Anyway, the above is more for future reference; I'll fix it up.

James

>  Signed-off-by: Justin P. Mattock <justinmattock@gmail.com>
> 
> ---
>  drivers/scsi/hosts.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
> index 6660fa9..4cc99b7 100644
> --- a/drivers/scsi/hosts.c
> +++ b/drivers/scsi/hosts.c
> @@ -325,7 +325,6 @@ struct Scsi_Host *scsi_host_alloc(struct scsi_host_template *sht, int privsize)
>  {
>  	struct Scsi_Host *shost;
>  	gfp_t gfp_mask = GFP_KERNEL;
> -	int rval;
>  
>  	if (sht->unchecked_isa_dma && privsize)
>  		gfp_mask |= __GFP_DMA;
> @@ -420,7 +419,8 @@ struct Scsi_Host *scsi_host_alloc(struct scsi_host_template *sht, int privsize)
>  	shost->ehandler = kthread_run(scsi_error_handler, shost,
>  			"scsi_eh_%d", shost->host_no);
>  	if (IS_ERR(shost->ehandler)) {
> -		rval = PTR_ERR(shost->ehandler);
> +		printk(KERN_WARNING "scsi%d: error handler thread failed to spawn, error = %ld\n", 
> +			shost->host_no, PTR_ERR(shost->ehandler));
>  		goto fail_kfree;
>  	}
>  

Re: [PATCH 5/5 v2]scsi:hosts.c Fix warning: variable 'rval' set but not used

[Justin P. Mattock]

On 06/18/2010 01:28 PM, James Bottomley wrote:
> On Fri, 2010-06-18 at 13:16 -0700, Justin P. Mattock wrote:
>> The below patch fixes a warning message generated by gcc 4.6.0
>>    CC      drivers/scsi/hosts.o
>> drivers/scsi/hosts.c: In function 'scsi_host_alloc':
>> drivers/scsi/hosts.c:328:6: warning: variable 'rval' set but not used
>
> OK, patch looks fine now, thanks!  But this isn't:
>
>>   Signed-off-by: James Bottomley<James.Bottomley@suse.de>
>
> You need to read Documentation/SubmittingPatches for the full story, but
> you can't just add signed-off-by; you need an explicit email from the
> person saying that ... plus, signoffs should follow the natural
> progression of the patch, so if it's your patch, and you send it to me,
> I'll add my signoff before I put it in the tree.  This has a legal basis
> in the DCO (see SubmittinPatches).  We're a lot looser on the various
> other tags, though.
>
> Anyway, the above is more for future reference; I'll fix it up.
>
> James

alright!! and Thanks for be patient with me...


>
>>   Signed-off-by: Justin P. Mattock<justinmattock@gmail.com>
>>
>> ---
>>   drivers/scsi/hosts.c |    4 ++--
>>   1 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
>> index 6660fa9..4cc99b7 100644
>> --- a/drivers/scsi/hosts.c
>> +++ b/drivers/scsi/hosts.c
>> @@ -325,7 +325,6 @@ struct Scsi_Host *scsi_host_alloc(struct scsi_host_template *sht, int privsize)
>>   {
>>   	struct Scsi_Host *shost;
>>   	gfp_t gfp_mask = GFP_KERNEL;
>> -	int rval;
>>
>>   	if (sht->unchecked_isa_dma&&  privsize)
>>   		gfp_mask |= __GFP_DMA;
>> @@ -420,7 +419,8 @@ struct Scsi_Host *scsi_host_alloc(struct scsi_host_template *sht, int privsize)
>>   	shost->ehandler = kthread_run(scsi_error_handler, shost,
>>   			"scsi_eh_%d", shost->host_no);
>>   	if (IS_ERR(shost->ehandler)) {
>> -		rval = PTR_ERR(shost->ehandler);
>> +		printk(KERN_WARNING "scsi%d: error handler thread failed to spawn, error = %ld\n",
>> +			shost->host_no, PTR_ERR(shost->ehandler));
>>   		goto fail_kfree;
>>   	}
>>
>
>
>

Re: [PATCH 5/5 v2]scsi:hosts.c Fix warning: variable 'rval' set but not used

[Matthew Wilcox]

On Fri, Jun 18, 2010 at 01:16:07PM -0700, Justin P. Mattock wrote:
> @@ -420,7 +419,8 @@ struct Scsi_Host *scsi_host_alloc(struct scsi_host_template *sht, int privsize)
>  	shost->ehandler = kthread_run(scsi_error_handler, shost,
>  			"scsi_eh_%d", shost->host_no);
>  	if (IS_ERR(shost->ehandler)) {
> -		rval = PTR_ERR(shost->ehandler);
> +		printk(KERN_WARNING "scsi%d: error handler thread failed to spawn, error = %ld\n", 
> +			shost->host_no, PTR_ERR(shost->ehandler));

Surely this should simply be:

		shost_printk(KERN_WARNING, shost, "error handler thread failed"
				"to spawn (%ld)\n", PTR_ERR(shost->ehandler));

>  		goto fail_kfree;
>  	}
>  
> -- 
> 1.7.1.rc1.21.gf3bd6
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Matthew Wilcox				Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

Re: [PATCH 5/5 v2]scsi:hosts.c Fix warning: variable 'rval' set but not used

[Justin P. Mattock]

On 06/18/2010 01:47 PM, Matthew Wilcox wrote:
> On Fri, Jun 18, 2010 at 01:16:07PM -0700, Justin P. Mattock wrote:
>> @@ -420,7 +419,8 @@ struct Scsi_Host *scsi_host_alloc(struct scsi_host_template *sht, int privsize)
>>   	shost->ehandler = kthread_run(scsi_error_handler, shost,
>>   			"scsi_eh_%d", shost->host_no);
>>   	if (IS_ERR(shost->ehandler)) {
>> -		rval = PTR_ERR(shost->ehandler);
>> +		printk(KERN_WARNING "scsi%d: error handler thread failed to spawn, error = %ld\n",
>> +			shost->host_no, PTR_ERR(shost->ehandler));
>
> Surely this should simply be:
>
> 		shost_printk(KERN_WARNING, shost, "error handler thread failed"
> 				"to spawn (%ld)\n", PTR_ERR(shost->ehandler));
>
>>   		goto fail_kfree;
>>   	}
>>
>> --
>> 1.7.1.rc1.21.gf3bd6
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>


the above change goes through without a warning message.. should I resend?

Justin P. Mattock

Re: [PATCH 5/5 v2]scsi:hosts.c Fix warning: variable 'rval' set but not used

[James Bottomley]

On Fri, 2010-06-18 at 14:47 -0600, Matthew Wilcox wrote:
> On Fri, Jun 18, 2010 at 01:16:07PM -0700, Justin P. Mattock wrote:
> > @@ -420,7 +419,8 @@ struct Scsi_Host *scsi_host_alloc(struct scsi_host_template *sht, int privsize)
> >  	shost->ehandler = kthread_run(scsi_error_handler, shost,
> >  			"scsi_eh_%d", shost->host_no);
> >  	if (IS_ERR(shost->ehandler)) {
> > -		rval = PTR_ERR(shost->ehandler);
> > +		printk(KERN_WARNING "scsi%d: error handler thread failed to spawn, error = %ld\n", 
> > +			shost->host_no, PTR_ERR(shost->ehandler));
> 
> Surely this should simply be:
> 
> 		shost_printk(KERN_WARNING, shost, "error handler thread failed"
> 				"to spawn (%ld)\n", PTR_ERR(shost->ehandler));

Actually, I'm wary of doing this: the device isn't fully initialised
(it's unparented and not added to the system) so that could cause
problems down the road for intelligent error gathering.  It's why we
tend to do the printk scsi%d (or dev_printk on the parent) thing up
until scsi_host_add().

James