Skip to contentSkip to conversation

scsi: lpfc_scsi: Remove unused variables

DISCUSSION Jonathan Nieder 4 participants 5 messages Jun 7, 2012 – Jun 11, 2012 1 message not in the archive SCSI

lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

Jonathan Nieder

author
#1 lore raw

Hi Richard,

Quoted 4 lines · Richard Yao

This is my first time patch submission. The commit message should explain the issue.

commit ba8c7a7a629b736fdc1abceaa3ec49f8820aec08

Quoted 8 lines
Author: Richard Yao <[email protected]>
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

#2 lore raw
Quoted 19 lines · Jonathan Nieder

Hi Richard,

Quoted 4 lines · Richard Yao

This is my first time patch submission. The commit message should explain the issue.

commit ba8c7a7a629b736fdc1abceaa3ec49f8820aec08

Quoted 8 lines
Author: Richard Yao <[email protected]>
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?

Quoted 6 lines

(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.

author
#3 lore raw
Quoted 2 lines · Richard Yao

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

#4 lore raw

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

Quoted 23 lines · Jonathan Nieder
Quoted 2 lines · Richard Yao

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

#5 lore raw
In reply to Jonathan Nieder
Quoted 11 lines · Jonathan Nieder

Hi Richard,

Quoted 7 lines · Richard Yao

This is my first time patch submission. The commit message should explain the issue.

commit ba8c7a7a629b736fdc1abceaa3ec49f8820aec08

Author: Richard Yao <[email protected]>
Date:   Thu Jun 7 01:44:31 2012 -0400
Quoted 8 lines
Quoted 5 lines

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