Discussion:
Trivial patches to silence -Wundef
(too old to reply)
Tim Rühsen
2017-04-05 13:47:03 UTC
Permalink
Raw Message
These patches silence tons of warnings here...

Regards, Tim
Eric Blake
2017-04-05 15:29:25 UTC
Permalink
Raw Message
Post by Tim Rühsen
These patches silence tons of warnings here...
In general, we do NOT try to make gnulib .c files '-Wundef'-clean. The
best solution is to not use -Wundef on gnulib compilation (even if you
want to use it on your .c files). Of course, that means that .h files
are in the gray area where we've taken patches to silence -Wundef
warnings in the past.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Tim Rühsen
2017-04-05 17:56:04 UTC
Permalink
Raw Message
Post by Eric Blake
Post by Tim Rühsen
These patches silence tons of warnings here...
In general, we do NOT try to make gnulib .c files '-Wundef'-clean. The
best solution is to not use -Wundef on gnulib compilation (even if you
want to use it on your .c files). Of course, that means that .h files
are in the gray area where we've taken patches to silence -Wundef
warnings in the past.
Hi Eric,

while i can (and do) give the gnulib files their own CFLAGS, I can't and won't
leave out -Wundef on my own C files. Since these include gnulib's header files,
I am currently swamped with -Wundef warnings. Especially when cross-compiling
for Windows. It is really pretty hard to manually sort out the 'relevant'
warnings between these. It also prevents compiling with -Werror which belongs
to "C best practices", e.g. CI immediately fails when a patch introduces new
warnings.

However, the patches solve all -Wundef issues at least for my project which
includes many gnulib modules. Also, changing #if to #ifdef at particular
places is no bloat or something, more of a (pedantic) cleanup, so it's
nothing bad.

So please consider at least the ENABLE_NLS patch to be applied, since the
related warnings pop up in almost every C file compiled. The OPENSSL related
warnings are less annoying, they just pop up once.

Regards, Tim
Paul Eggert
2017-04-05 21:08:21 UTC
Permalink
Raw Message
Post by Tim Rühsen
while i can (and do) give the gnulib files their own CFLAGS, I can't and won't
leave out -Wundef on my own C files.
You can use something like this in your source files:

#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wundef"
#include <some-gnulib-file.h>
#pragma GCC diagnostic pop
Tim Rühsen
2017-04-06 08:29:11 UTC
Permalink
Raw Message
Post by Paul Eggert
#pragma GCC diagnostic ignored "-Wundef"
Thank you.

Though I am aware of the diagnostic pragma, it needed your hint to have
a closer look... it turned out I just have to change on central header
file :-) (First I thought I had to do the change in each C file).

Regards, Tim
Bruno Haible
2017-04-05 20:59:09 UTC
Permalink
Raw Message
Hi Tim,
Post by Tim Rühsen
These patches silence tons of warnings here...
0001-lib-gettext.h-Avoid-Wundef-warningi-for-ENABLE_NLS.patch
We cannot apply this patch. While gettext.m4 defines ENABLE_NLS to 1
or doesn't define it at all, many developers use "#defined ENABLE_NLS 0"
to disable NLS from GNU gettext: A web search for "define enable_nls 0"
lists 133 hits.
The proposed patch would break these uses.

Bruno
Tim Rühsen
2017-04-06 07:53:49 UTC
Permalink
Raw Message
Post by Bruno Haible
Hi Tim,
Post by Tim Rühsen
These patches silence tons of warnings here...
0001-lib-gettext.h-Avoid-Wundef-warningi-for-ENABLE_NLS.patch
We cannot apply this patch. While gettext.m4 defines ENABLE_NLS to 1
or doesn't define it at all, many developers use "#defined ENABLE_NLS 0"
to disable NLS from GNU gettext: A web search for "define enable_nls 0"
lists 133 hits.
The proposed patch would break these uses.
True, I missed that.

Here is the amended patch to deal with that situation.

Regards, Tim
Bruno Haible
2017-04-21 23:39:39 UTC
Permalink
Raw Message
Post by Tim Rühsen
Here is the amended patch to deal with that situation.
Thanks. I amended the comment as well:


2017-04-21 Bruno Haible <***@clisp.org>

gettext-h: Avoid -Wundef warning.
* lib/gettext.h: Test the value of ENABLE_NLS only if it is defined.
Reported by Tim Rühsen <***@gmx.de> in
<https://lists.gnu.org/archive/html/bug-gnulib/2017-04/msg00022.html>.

diff --git a/lib/gettext.h b/lib/gettext.h
index 888e2fc..2a12bb6 100644
--- a/lib/gettext.h
+++ b/lib/gettext.h
@@ -18,8 +18,9 @@
#ifndef _LIBGETTEXT_H
#define _LIBGETTEXT_H 1

-/* NLS can be disabled through the configure --disable-nls option. */
-#if ENABLE_NLS
+/* NLS can be disabled through the configure --disable-nls option
+ or through "#define ENABLE NLS 0" before including this file. */
+#if defined ENABLE_NLS && ENABLE_NLS

/* Get declarations of GNU message catalog functions. */
# include <libintl.h>
Gisle Vanem
2017-04-22 09:00:32 UTC
Permalink
Raw Message
Post by Bruno Haible
diff --git a/lib/gettext.h b/lib/gettext.h
index 888e2fc..2a12bb6 100644
--- a/lib/gettext.h
+++ b/lib/gettext.h
@@ -18,8 +18,9 @@
#ifndef _LIBGETTEXT_H
#define _LIBGETTEXT_H 1
-/* NLS can be disabled through the configure --disable-nls option. */
-#if ENABLE_NLS
+/* NLS can be disabled through the configure --disable-nls option
+ or through "#define ENABLE NLS 0" before including this file. */
+#if defined ENABLE_NLS && ENABLE_NLS
Speaking of gettext, shouldn't lib/xbinary-io.c test on ENABLE_NLS
too? Because now I get:
xbinary-io.c(34): warning C4047: 'function': 'const char *' differs in levels of indirection from 'int'
xbinary-io.c(34): warning C4024: 'error': different types for formal and actual parameter 3

Where is '_(x)' defined if ENABLE_NLS is not defined?
--
--gv
Bruno Haible
2017-04-22 10:11:56 UTC
Permalink
Raw Message
Hi,
Post by Gisle Vanem
xbinary-io.c(34): warning C4047: 'function': 'const char *' differs in levels of indirection from 'int'
xbinary-io.c(34): warning C4024: 'error': different types for formal and actual parameter 3
Fixed by the patch below. Thanks for reporting this.
Post by Gisle Vanem
Where is '_(x)' defined if ENABLE_NLS is not defined?
_(x) ought to be defined by the file that uses it.
ENABLE_NLS does not need to be considered by this file; it is already taken
into account by gettext.h.


2017-04-22 Bruno Haible <***@clisp.org>

xbinary-io: Fix build error.
* modules/xbinary-io (Depends-on): Add gettext-h.
* lib/xbinary-io.c: Include gettext.h and define _().
Reported by Gisle Vanem <***@gmail.com> in
<https://lists.gnu.org/archive/html/bug-gnulib/2017-04/msg00089.html>.

diff --git a/modules/xbinary-io b/modules/xbinary-io
index 5344805..7618e37 100644
--- a/modules/xbinary-io
+++ b/modules/xbinary-io
@@ -10,6 +10,7 @@ binary-io
error
exitfail
extern-inline
+gettext-h
stdbool
verify

diff --git a/lib/xbinary-io.c b/lib/xbinary-io.c
index 7f765f1..b51566b 100644
--- a/lib/xbinary-io.c
+++ b/lib/xbinary-io.c
@@ -25,6 +25,9 @@
#include "exitfail.h"
#include "verify.h"

+#include "gettext.h"
+#define _(msgid) gettext (msgid)
+
#if O_BINARY

_Noreturn void

Loading...