Re: [patch] cleanup: Wunused corefile.c

Aleksandar Ristovski
2013-01-31T20:29:58Z

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