GNOME Bugzilla – Bug 654572
Set but unused warning
Last modified: 2011-07-20 17:12:59 UTC
I came across couple of case of set but unused variable warning preventing compilation of gst-plugins-bad: http://cgit.collabora.com/git/user/nicolas/gst-plugins-bad.git/log/?h=unused
These can all be one commit. Please just delete code, don't disable it using /* */. (*Especially* don't use /* */, as that is for comments. Use #if 0 or #ifdef unused to disable code, that way indent still works correctly.)
Edward told me exactly the opposite! (to comment out, not remove code)
Then use #if 0 and not comments. The point is not to lose information/code which is presently unused but would be potentially useful later on. Also ... it would be good to dig in a bit deeper than "oh, variable unused, let's remove it". There's a chance that it's also the indicator of a bug in the code. I would have removed all of those a year ago when I did the big clang analysis, but didn't because I wasn't 100% sure they were *really* unused. In case of doubt, I'd much prefer if you contacted the maintainers/authors of those plugins to make sure those variables are really not needed.
I don't mind removing that pretty clearly looks unused. The code can always be retrieved from git. Edward is correct that we prefer to use #ifdef unused or #if 0 for larger chunks that are accidentally unused because of a bug -- removing the code papers over the bug. In any case, this is what code review is for. If I don't approve of deleting a chunk of code, I'll just remove it from the commit before pushing.
That clarify a bit the what to do. For assrender, ignoring the stride of an image seems wrong, so I'd suggest commenting out with FIXME and a bug id to go with it. The caps in lv2, I think they can simply be removed. For the timestamp in modplug I just have no idea, thus if 0 might be better. But I don't see obvious reason to file a bug. In sndfile, there is good chance that the dicount boolean should result in setting discount flag on the created buffer, but again I don't have any context. Finally the rest seems correct fix to me, like the wildmidi fix where stuff in ifdef had variable declaration outside those ifdef. So I suggest I squash all this, replace the /* */ with ifdef, file a bug for assrender and set discount flag in sndfile. Those that make sense ?
Created attachment 192213 [details] [review] Fix unused but set
Committed commit e8d24859ca5629198c581b570402e9a459839d7c Author: Nicolas Dufresne <nicolas.dufresne@collabora.com> Date: Mon Jul 18 14:53:31 2011 -0400 Fix compilation for unused but not set https://bugzilla.gnome.org/show_bug.cgi?id=654572