Re: [patch] cleanup: Wunused corefile.c

[Aleksandar Ristovski <aristovski at qnx dot com>]

On 13-01-31 03:24 PM, Tom Tromey wrote:
> > > > > > "Aleksandar" == Aleksandar Ristovski <aristovski@qnx.com> writes:
>
> Aleksandar> In addition to already posted/committed Wunused I have a bunch of
> Aleksandar> patches which work around unused vars by adding attribute unused.
>
> You should probably try a --enable-targets=all build if you want to
> enable -Wunused in configure...
>
> Aleksandar> Rationale was: either it was unclear whether the assignment
> Aleksandar> might have side-effects or there was something that should
> Aleksandar> have been done (e.g. this case) with the variable.
>
> Aleksandar> Let me know if this is acceptable approach.
>
> It definitely isn't in this form, and perhaps not in other forms.
>
> First, in this case, the is just buggy.  If stat fails, then st.st_mtime
> isn't necessarily set, and so the subsequent test is reading garbage.
> This is the sort of bug that -Wunused helps to diagnose -- so adding an
> attribute to silence the error isn't what we should do.
>
> Second, using __attribute__ unconditionally isn't ok.  We can use the
> ATTRIBUTE_UNUSED macro; but I would imagine in most cases it would be
> better to fix the problem in some other way.

This is why I put a FIXME - intending to revisit and properly fix.

But there are other cases where it is not about missing code, but e.g. 
conditional compilation. For example:

Index: gdb/inflow.c
===================================================================
RCS file: /cvs/src/src/gdb/inflow.c,v
retrieving revision 1.69
diff -u -p -r1.69 inflow.c
--- gdb/inflow.c        1 Jan 2013 06:32:45 -0000       1.69
+++ gdb/inflow.c        30 Jan 2013 22:25:15 -0000
@@ -397,7 +397,7 @@ terminal_ours_1 (int output_only)
          pgrp.  */
       void (*osigttou) () = NULL;
 #endif
-      int result;
+      int result __attribute__ ((unused));

 #ifdef SIGTTOU
       if (job_control)


However, I do not intend to push this very hard due to lack of time. I 
simply did a swipe over the code and made it compile with Wunused 
(without regressions) on x86_64-linux-gnu and wanted to contribute as 
much as I can without spending too much time on it.


Thanks again,

Aleksandar