Re: scsi: lpfc_scsi: Remove unused variables

Re: scsi: lpfc_scsi: Remove unused variables

[Jonathan Nieder]

Hi Richard,

Richard Yao wrote:

> This is my first time patch submission. The commit message should
> explain the issue.
>
> commit ba8c7a7a629b736fdc1abceaa3ec49f8820aec08
> Author: Richard Yao <ryao@cs.stonybrook.edu>
> Date:   Thu Jun 7 01:44:31 2012 -0400
>
>     scsi: lpfc_scsi: Remove unused variables
>
>     Kernels built with CONFIG_SCSI_LPFC fail to compile due to the use of
>     -Werror=unused-variable. We solve that by removing some unused
>     variables.

Wouldn't a saner fix be to stop using -Werror=unused-variable, or am I
missing something subtle?

(Since v3.4-rc1~6^2~48 the makefile seems to use plain -Werror, which
seems even worse, since it means any random compiler bug that triggers
a new warning will break the build.)

Thanks and hope that helps,
Jonathan

Re: scsi: lpfc_scsi: Remove unused variables

[Richard Yao]

On 06/07/12 02:12, Jonathan Nieder wrote:
> Hi Richard,
>
> Richard Yao wrote:
>
>> This is my first time patch submission. The commit message should
>> explain the issue.
>>
>> commit ba8c7a7a629b736fdc1abceaa3ec49f8820aec08
>> Author: Richard Yao <ryao@cs.stonybrook.edu>
>> Date:   Thu Jun 7 01:44:31 2012 -0400
>>
>>     scsi: lpfc_scsi: Remove unused variables
>>
>>     Kernels built with CONFIG_SCSI_LPFC fail to compile due to the use of
>>     -Werror=unused-variable. We solve that by removing some unused
>>     variables.
> Wouldn't a saner fix be to stop using -Werror=unused-variable, or am I
> missing something subtle?
>
> (Since v3.4-rc1~6^2~48 the makefile seems to use plain -Werror, which
> seems even worse, since it means any random compiler bug that triggers
> a new warning will break the build.)
>
> Thanks and hope that helps,
> Jonathan

Using -Werror enables us to catch potential runtime issues before they
happen. This one is not a runtime issue, but the compiler is right in
stating that these variables serve no purpose. My opinion is that
occasional noise due to -Werror won't be a problem. Also, decisions on
-Werror probably should be made by either the subsystem maintainer or
the branch maintainer.

Re: scsi: lpfc_scsi: Remove unused variables

[Greg KH]

On Thu, Jun 07, 2012 at 01:12:56AM -0500, Jonathan Nieder wrote:
> Hi Richard,
> 
> Richard Yao wrote:
> 
> > This is my first time patch submission. The commit message should
> > explain the issue.
> >
> > commit ba8c7a7a629b736fdc1abceaa3ec49f8820aec08
> > Author: Richard Yao <ryao@cs.stonybrook.edu>
> > Date:   Thu Jun 7 01:44:31 2012 -0400
> >
> >     scsi: lpfc_scsi: Remove unused variables
> >
> >     Kernels built with CONFIG_SCSI_LPFC fail to compile due to the use of
> >     -Werror=unused-variable. We solve that by removing some unused
> >     variables.
> 
> Wouldn't a saner fix be to stop using -Werror=unused-variable, or am I
> missing something subtle?

Yes, that is correct, older versions of gcc, and sometimes newer ones,
get this logic wrong and incorrectly flag variables that really are used
due to bugs in their detection logic.  Don't require a developer to have
a gcc version that is different from what the rest of the kernel
requires, as that is not allowed.

thanks,

greg k-h

Re: scsi: lpfc_scsi: Remove unused variables

[Jonathan Nieder]

Richard Yao wrote:

> Using -Werror enables us to catch potential runtime issues before they
> happen.

You have more trust in GCC and other compilers than I do.

If you'd like some entertainment, then for example see:

 http://gcc.gnu.org/PR52798
 http://gcc.gnu.org/PR25509

Bugs and misdesigned features do not have to be involved for this to
be a problem --- all it takes is a gcc upgrade that adds a new
warning, and suddenly you've turned your existing potential code
cleanliness improvements into failed builds and lots of wasted time
for innocent sysadmins.  -Werror is great for development, but using
it in production turns out to be pretty much always a terrible idea.

Thanks for the chance to vent about this. :)  It's bad, bad, bad.

Ciao,
Jonathan

Re: scsi: lpfc_scsi: Remove unused variables

[James Smart]

Richard,

fyi - this error in lpfc was corrected in our 8.3.31 patch set - posted 
back in May:
http://www.spinics.net/lists/linux-scsi/msg59216.html

-- james


On 6/7/2012 2:42 AM, Jonathan Nieder wrote:
> Richard Yao wrote:
>
>> Using -Werror enables us to catch potential runtime issues before they
>> happen.
> You have more trust in GCC and other compilers than I do.
>
> If you'd like some entertainment, then for example see:
>
>   http://gcc.gnu.org/PR52798
>   http://gcc.gnu.org/PR25509
>
> Bugs and misdesigned features do not have to be involved for this to
> be a problem --- all it takes is a gcc upgrade that adds a new
> warning, and suddenly you've turned your existing potential code
> cleanliness improvements into failed builds and lots of wasted time
> for innocent sysadmins.  -Werror is great for development, but using
> it in production turns out to be pretty much always a terrible idea.
>
> Thanks for the chance to vent about this. :)  It's bad, bad, bad.
>
> Ciao,
> Jonathan
>