diff mbox

In debug/tst-chk1 we purposely test gets and getwd?

Message ID 540D0803.5030502@redhat.com
State Not applicable
Headers

Commit Message

Carlos O'Donell Sept. 8, 2014, 1:36 a.m. UTC 
  It seems in debug/tst-chk1.c we purposely test gets
and getwd, but both of those trigger deprecation
warnings. It would be nice to eventually turn on 
-Werror.

To fix the warnings for debug/tstk-chk1.c would we
use something like this?

---

I haven't checked what version of gcc added these
pragmas though, so I don't know what build version of
gcc we'd have to bump up to. I'm just looking at the
long term picture.

Cheers,
Carlos.

Comments

Joseph Myers Sept. 8, 2014, 3:28 p.m. UTC | #1 
On Sun, 7 Sep 2014, Carlos O'Donell wrote:

> It seems in debug/tst-chk1.c we purposely test gets
> and getwd, but both of those trigger deprecation
> warnings. It would be nice to eventually turn on 
> -Werror.

Yes, for various testcases it will be necessary to disable some warnings, 
or to use -Wno-error= for them, because tests need to cover code using 
deprecated interfaces, and code doing bad things that can be detected at 
compile time (e.g. some _FORTIFY_SOURCE tests).

There are already various -Wno- settings in the makefiles which it may 
make sense to review to see if they are still necessary or if there are 
better ways to address those warnings (if still present) - we'll need to 
work out our policy on when to use such settings, and how to handle 
hard-to-fix warnings in general, when using -Werror by default.

(At one point I thought maybe we should enable -Werror at first only for 
installed code, not tests.  But I now think it would be simpler to enable 
it everywhere and then selectively disable it for particular tests; 
probably most warnings in tests are in fact easy to fix without affecting 
what's being tested.)
Carlos O'Donell Sept. 8, 2014, 3:37 p.m. UTC | #2 
On 09/08/2014 11:28 AM, Joseph S. Myers wrote:
> On Sun, 7 Sep 2014, Carlos O'Donell wrote:
> 
>> It seems in debug/tst-chk1.c we purposely test gets
>> and getwd, but both of those trigger deprecation
>> warnings. It would be nice to eventually turn on 
>> -Werror.
> 
> Yes, for various testcases it will be necessary to disable some warnings, 
> or to use -Wno-error= for them, because tests need to cover code using 
> deprecated interfaces, and code doing bad things that can be detected at 
> compile time (e.g. some _FORTIFY_SOURCE tests).
> 
> There are already various -Wno- settings in the makefiles which it may 
> make sense to review to see if they are still necessary or if there are 
> better ways to address those warnings (if still present) - we'll need to 
> work out our policy on when to use such settings, and how to handle 
> hard-to-fix warnings in general, when using -Werror by default.
> 
> (At one point I thought maybe we should enable -Werror at first only for 
> installed code, not tests.  But I now think it would be simpler to enable 
> it everywhere and then selectively disable it for particular tests; 
> probably most warnings in tests are in fact easy to fix without affecting 
> what's being tested.)

Thanks for that feedback.

Any particular opposition to #pragma usage? It seems like easier maintenance
to add the #pragma's close to their point of use with comments talking about
why we avoid the warning.

Cheers,
Carlos.
Roland McGrath Sept. 9, 2014, 9:19 p.m. UTC | #3 
4.4 supports '#pragma GCC diagnostic ignored "..."', but only at top level,
and it does not support '#pragma GCC push'.  Even a file-wide pragma seems
preferable to using command-line options (and we should clean up existing
cases of 'CFLAGS-foo.c = -Wno-...').  But perhaps before we get to this,
we'll decide to bump the minimum compiler up to 4.6, and then we could use
tight push/pop regions to disable around just the one line, which is better.
Carlos O'Donell Sept. 10, 2014, 5:12 p.m. UTC | #4 
On 09/09/2014 05:19 PM, Roland McGrath wrote:
> 4.4 supports '#pragma GCC diagnostic ignored "..."', but only at top level,
> and it does not support '#pragma GCC push'.  Even a file-wide pragma seems
> preferable to using command-line options (and we should clean up existing
> cases of 'CFLAGS-foo.c = -Wno-...').  But perhaps before we get to this,
> we'll decide to bump the minimum compiler up to 4.6, and then we could use
> tight push/pop regions to disable around just the one line, which is better.

Awesome, so I'll hold off on this until we discuss 4.6 usage... or rather
I'll start that discussion and see where it goes.

c.
diff mbox

Patch

diff --git a/debug/tst-chk1.c b/debug/tst-chk1.c
index 3393153..2262cc4 100644
--- a/debug/tst-chk1.c
+++ b/debug/tst-chk1.c
@@ -730,6 +730,9 @@  do_test (void)
       exit (1);
     }
 
+  /* We purposely test gets, so ignore the warnings.  */
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
   if (gets (buf) != buf || memcmp (buf, "abcdefgh", 9))
     FAIL ();
   if (gets (buf) != buf || memcmp (buf, "ABCDEFGHI", 10))
@@ -741,6 +744,7 @@  do_test (void)
     FAIL ();
   CHK_FAIL_END
 #endif
+#pragma GCC diagnostic pop
 
   rewind (stdin);
 
@@ -1144,6 +1148,9 @@  do_test (void)
          CHK_FAIL_END
 #endif
 
+         /* We purposely test getwd, so ignore the warnings.  */
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
          if (getwd (getcwdbuf) != getcwdbuf
              || strcmp (getcwdbuf, fname) != 0)
            FAIL ();
@@ -1158,6 +1165,7 @@  do_test (void)
            FAIL ();
          CHK_FAIL_END
 #endif
+#pragma GCC diagnostic pop
        }
 
       if (chdir (cwd1) != 0)