| Message ID | 540D0803.5030502@redhat.com |
|---|---|
| State | New |
| Headers | show |
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.)
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.
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.
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 --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)