Here are a few warning-removal changes for daemon/:
From eea38a1937ccd396603ed899d829d2982755ea4a Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering@redhat.com> Date: Thu, 6 Aug 2009 15:25:03 +0200 Subject: [PATCH 1/4] define ATTRIBUTE_UNUSED
* daemon/daemon.h (__attribute__, ATTRIBUTE_UNUSED): Define. --- daemon/daemon.h | 10 ++++++++++ 1 files changed, 10 insertions(+), 0 deletions(-) diff --git a/daemon/daemon.h b/daemon/daemon.h index 70a2351..a6bbb73 100644 --- a/daemon/daemon.h +++ b/daemon/daemon.h @@ -213,4 +213,14 @@ extern void reply (xdrproc_t xdrp, char *ret); } \ while (0) +#ifndef __attribute__ +# if __GNUC__ < 2 || (__GNUC__ == 2 && __GNUC_MINOR__ < 8) +# define __attribute__(x) /* empty */ +# endif +#endif + +#ifndef ATTRIBUTE_UNUSED +# define ATTRIBUTE_UNUSED __attribute__ ((__unused__)) +#endif + #endif /* GUESTFSD_DAEMON_H */ -- 1.6.4.174.gc193a
From af4e98c3371f6008188fb3ef7136c1ebda008933 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering@redhat.com> Date: Thu, 6 Aug 2009 15:26:13 +0200 Subject: [PATCH 2/4] build: avoid warnings in daemon/debug.c
* daemon/debug.c (do_debug): Mark parameters as unused. --- daemon/debug.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/daemon/debug.c b/daemon/debug.c index 13b7240..5b3b5fa 100644 --- a/daemon/debug.c +++ b/daemon/debug.c @@ -61,7 +61,7 @@ static struct cmd cmds[] = { #endif char * -do_debug (char *subcmd, char **argv) +do_debug (char *subcmd ATTRIBUTE_UNUSED, char **argv ATTRIBUTE_UNUSED) { #if ENABLE_DEBUG_COMMAND int argc, i; -- 1.6.4.174.gc193a
From a174e20227e5299a172934d9f60023ac55530a1f Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering@redhat.com> Date: Thu, 6 Aug 2009 15:28:48 +0200 Subject: [PATCH 3/4] build: avoid warnings in daemon/guestfsd.c
* daemon/guestfsd.c (print_arginfo, print_shell_quote): Mark each "info" parameter as unused. --- daemon/guestfsd.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/daemon/guestfsd.c b/daemon/guestfsd.c index 07a1c5e..07a952f 100644 --- a/daemon/guestfsd.c +++ b/daemon/guestfsd.c @@ -718,7 +718,7 @@ split_lines (char *str) */ static int print_shell_quote (FILE *stream, - const struct printf_info *info, + const struct printf_info *info ATTRIBUTE_UNUSED, const void *const *args) { #define SAFE(c) (isalnum((c)) || \ @@ -751,7 +751,7 @@ print_sysroot_shell_quote (FILE *stream, #ifdef HAVE_REGISTER_PRINTF_SPECIFIER static int -print_arginfo (const struct printf_info *info, +print_arginfo (const struct printf_info *info ATTRIBUTE_UNUSED, size_t n, int *argtypes, int *size) { if (n > 0) { -- 1.6.4.174.gc193a
From b9842a2ca010bf89b11b4b640bbdee5df58f8341 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering@redhat.com> Date: Thu, 6 Aug 2009 16:05:44 +0200 Subject: [PATCH 4/4] build: avoid warnings in daemon/inotify.c
Avoid "comparison between signed and unsigned integer expressions" warnings. If it's at all hard or risky to avoid this type of warning, then it's not worthwhile. Here, it's easy and safe. * daemon/inotify.c (inotify_posn): Declare local to be of unsigned type. (do_inotify_read, do_inotify_files): Likewise. --- daemon/inotify.c | 7 ++++--- 1 files changed, 4 insertions(+), 3 deletions(-) diff --git a/daemon/inotify.c b/daemon/inotify.c index 1b90f2b..8bb8ed2 100644 --- a/daemon/inotify.c +++ b/daemon/inotify.c @@ -33,7 +33,7 @@ static int inotify_fd = -1; static char inotify_buf[64*1024*1024]; /* Event buffer, [0..posn-1] is valid */ -static int inotify_posn = 0; +static size_t inotify_posn = 0; /* Because inotify_init does NEED_ROOT, NEED_INOTIFY implies NEED_ROOT. */ #define NEED_INOTIFY(errcode) \ @@ -184,7 +184,8 @@ do_inotify_read (void) while (space > 0) { struct inotify_event *event; - int n, r; + int r; + size_t n; r = read (inotify_fd, inotify_buf + inotify_posn, sizeof (inotify_buf) - inotify_posn); @@ -274,7 +275,7 @@ do_inotify_files (void) { char **ret = NULL; int size = 0, alloc = 0; - int i; + unsigned int i; FILE *fp; guestfs_int_inotify_event_list *events; char buf[PATH_MAX]; -- 1.6.4.174.gc193a
On 10/08/09 09:57, Jim Meyering wrote:
Here are a few warning-removal changes for daemon/:
From eea38a1937ccd396603ed899d829d2982755ea4a Mon Sep 17 00:00:00 2001 From: Jim Meyering<meyering@redhat.com> Date: Thu, 6 Aug 2009 15:25:03 +0200 Subject: [PATCH 1/4] define ATTRIBUTE_UNUSED
* daemon/daemon.h (__attribute__, ATTRIBUTE_UNUSED): Define. --- daemon/daemon.h | 10 ++++++++++ 1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/daemon/daemon.h b/daemon/daemon.h index 70a2351..a6bbb73 100644 --- a/daemon/daemon.h +++ b/daemon/daemon.h @@ -213,4 +213,14 @@ extern void reply (xdrproc_t xdrp, char *ret); } \ while (0)
+#ifndef __attribute__ +# if __GNUC__< 2 || (__GNUC__ == 2&& __GNUC_MINOR__< 8) +# define __attribute__(x) /* empty */ +# endif +#endif + +#ifndef ATTRIBUTE_UNUSED +# define ATTRIBUTE_UNUSED __attribute__ ((__unused__)) +#endif + #endif /* GUESTFSD_DAEMON_H */ -- 1.6.4.174.gc193a
From af4e98c3371f6008188fb3ef7136c1ebda008933 Mon Sep 17 00:00:00 2001 From: Jim Meyering<meyering@redhat.com> Date: Thu, 6 Aug 2009 15:26:13 +0200 Subject: [PATCH 2/4] build: avoid warnings in daemon/debug.c
* daemon/debug.c (do_debug): Mark parameters as unused. --- daemon/debug.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/daemon/debug.c b/daemon/debug.c index 13b7240..5b3b5fa 100644 --- a/daemon/debug.c +++ b/daemon/debug.c @@ -61,7 +61,7 @@ static struct cmd cmds[] = { #endif
char * -do_debug (char *subcmd, char **argv) +do_debug (char *subcmd ATTRIBUTE_UNUSED, char **argv ATTRIBUTE_UNUSED) { #if ENABLE_DEBUG_COMMAND int argc, i; -- 1.6.4.174.gc193a
What warnings are you turning on? I normally compile with -Wall, and I don't get warning for unused function arguments: [mbooth@mbooth daemon]$ make CFLAGS="-g -Wall" debug.o gcc -DHAVE_CONFIG_H -I. -I./lib -Ilib -g -Wall -MT debug.o -MD -MP -MF .deps/debug.Tpo -c -o debug.o debug.c mv -f .deps/debug.Tpo .deps/debug.Po
From a174e20227e5299a172934d9f60023ac55530a1f Mon Sep 17 00:00:00 2001 From: Jim Meyering<meyering@redhat.com> Date: Thu, 6 Aug 2009 15:28:48 +0200 Subject: [PATCH 3/4] build: avoid warnings in daemon/guestfsd.c
* daemon/guestfsd.c (print_arginfo, print_shell_quote): Mark each "info" parameter as unused. --- daemon/guestfsd.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/daemon/guestfsd.c b/daemon/guestfsd.c index 07a1c5e..07a952f 100644 --- a/daemon/guestfsd.c +++ b/daemon/guestfsd.c @@ -718,7 +718,7 @@ split_lines (char *str) */ static int print_shell_quote (FILE *stream, - const struct printf_info *info, + const struct printf_info *info ATTRIBUTE_UNUSED, const void *const *args) { #define SAFE(c) (isalnum((c)) || \ @@ -751,7 +751,7 @@ print_sysroot_shell_quote (FILE *stream,
#ifdef HAVE_REGISTER_PRINTF_SPECIFIER static int -print_arginfo (const struct printf_info *info, +print_arginfo (const struct printf_info *info ATTRIBUTE_UNUSED, size_t n, int *argtypes, int *size) { if (n> 0) { -- 1.6.4.174.gc193a
From b9842a2ca010bf89b11b4b640bbdee5df58f8341 Mon Sep 17 00:00:00 2001 From: Jim Meyering<meyering@redhat.com> Date: Thu, 6 Aug 2009 16:05:44 +0200 Subject: [PATCH 4/4] build: avoid warnings in daemon/inotify.c
Avoid "comparison between signed and unsigned integer expressions" warnings. If it's at all hard or risky to avoid this type of warning, then it's not worthwhile. Here, it's easy and safe. * daemon/inotify.c (inotify_posn): Declare local to be of unsigned type. (do_inotify_read, do_inotify_files): Likewise. --- daemon/inotify.c | 7 ++++--- 1 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/daemon/inotify.c b/daemon/inotify.c index 1b90f2b..8bb8ed2 100644 --- a/daemon/inotify.c +++ b/daemon/inotify.c @@ -33,7 +33,7 @@ static int inotify_fd = -1;
static char inotify_buf[64*1024*1024]; /* Event buffer, [0..posn-1] is valid */ -static int inotify_posn = 0; +static size_t inotify_posn = 0;
/* Because inotify_init does NEED_ROOT, NEED_INOTIFY implies NEED_ROOT. */ #define NEED_INOTIFY(errcode) \ @@ -184,7 +184,8 @@ do_inotify_read (void)
while (space> 0) { struct inotify_event *event; - int n, r; + int r; + size_t n;
r = read (inotify_fd, inotify_buf + inotify_posn, sizeof (inotify_buf) - inotify_posn); @@ -274,7 +275,7 @@ do_inotify_files (void) { char **ret = NULL; int size = 0, alloc = 0; - int i; + unsigned int i; FILE *fp; guestfs_int_inotify_event_list *events; char buf[PATH_MAX]; -- 1.6.4.174.gc193a
Same here, I don't get any warnings. In general, I would like to get down to no compiler warnings. The biggest culprit in daemon, though, is rpcgen generated stuff. Matt -- Matthew Booth, RHCA, RHCSS Red Hat Engineering, Virtualisation Team M: +44 (0)7977 267231 GPG ID: D33C3490 GPG FPR: 3733 612D 2D05 5458 8A8A 1600 3441 EA19 D33C 3490
On 10/08/09 10:21, Jim Meyering wrote:
Matthew Booth wrote:
On 10/08/09 09:57, Jim Meyering wrote:
Here are a few warning-removal changes for daemon/: ... What warnings are you turning on? I normally compile with -Wall, and I don't get warning for unused function arguments:
I started with these: -Wall -W -Wextra
Now I see where you're coming from. ACK. Do you find it's generally useful to turn these on across the board? In my experience, until you get down to zero compiler warnings and turn on -Werror, they often go unnoticed. Are we likely to be able to get down to zero compiler warnings? Matt -- Matthew Booth, RHCA, RHCSS Red Hat Engineering, Virtualisation Team M: +44 (0)7977 267231 GPG ID: D33C3490 GPG FPR: 3733 612D 2D05 5458 8A8A 1600 3441 EA19 D33C 3490
On Mon, Aug 10, 2009 at 10:26:03AM +0100, Matthew Booth wrote:
On 10/08/09 10:21, Jim Meyering wrote:
Matthew Booth wrote:
On 10/08/09 09:57, Jim Meyering wrote:
Here are a few warning-removal changes for daemon/: ... What warnings are you turning on? I normally compile with -Wall, and I don't get warning for unused function arguments:
I started with these: -Wall -W -Wextra
Now I see where you're coming from. ACK.
Do you find it's generally useful to turn these on across the board? In my experience, until you get down to zero compiler warnings and turn on -Werror, they often go unnoticed. Are we likely to be able to get down to zero compiler warnings?
I don't see why not - libvirt managed to get there, even for the crazy mingw32 compile. Once there, development should definitely be done using -Werror on at all times. Also using the new AM_SILENT_RULES([yes]) lets you quite clearly see compile warning even without -Werror Regards, Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
Matthew Booth wrote:
On 10/08/09 10:21, Jim Meyering wrote:
Matthew Booth wrote:
On 10/08/09 09:57, Jim Meyering wrote:
Here are a few warning-removal changes for daemon/: ... What warnings are you turning on? I normally compile with -Wall, and I don't get warning for unused function arguments:
I started with these: -Wall -W -Wextra
Now I see where you're coming from. ACK.
Do you find it's generally useful to turn these on across the board?
Yes, Very.
In my experience, until you get down to zero compiler warnings and turn on -Werror, they often go unnoticed. Are we likely to be able to get down to zero compiler warnings?
Yes. ;-)
Jim Meyering wrote:
Matthew Booth wrote:
On 10/08/09 10:21, Jim Meyering wrote:
Matthew Booth wrote:
On 10/08/09 09:57, Jim Meyering wrote:
Here are a few warning-removal changes for daemon/: ... What warnings are you turning on? I normally compile with -Wall, and I don't get warning for unused function arguments:
I started with these: -Wall -W -Wextra
Now I see where you're coming from. ACK.
Do you find it's generally useful to turn these on across the board?
Yes, Very.
Though I should qualify that. It's a delicate balance. The following is the configure.ac snippet that has evolved for coreutils: The idea is to enable as all warnings, and to go through correcting offending code or disabling individual warnings, as taste and time permit. Eventually, you learn that certain classes of warnings are not worth trying to avoid, because the cure is worse than the disease. For others, the added value appears minimal, so not worth much time. AC_ARG_ENABLE([gcc-warnings], [AS_HELP_STRING([--enable-gcc-warnings], [turn on lots of GCC warnings (for developers)])], [case $enableval in yes|no) ;; *) AC_MSG_ERROR([bad value $enableval for gcc-warnings option]) ;; esac gl_gcc_warnings=$enableval], [gl_gcc_warnings=no] ) if test "$gl_gcc_warnings" = yes; then gl_WARN_ADD([-Werror], [WERROR_CFLAGS]) AC_SUBST([WERROR_CFLAGS]) nw= # This, $nw, is the list of warnings we disable. nw="$nw -Wdeclaration-after-statement" # too useful to forbid nw="$nw -Waggregate-return" # anachronistic nw="$nw -Wlong-long" # C90 is anachronistic (lib/gethrxtime.h) nw="$nw -Wc++-compat" # We don't care about C++ compilers nw="$nw -Wundef" # Warns on '#if GNULIB_FOO' etc in gnulib nw="$nw -Wtraditional" # Warns on #elif which we use often nw="$nw -Wcast-qual" # Too many warnings for now nw="$nw -Wconversion" # Too many warnings for now nw="$nw -Wsystem-headers" # Don't let system headers trigger warnings nw="$nw -Wsign-conversion" # Too many warnings for now nw="$nw -Wtraditional-conversion" # Too many warnings for now nw="$nw -Wunreachable-code" # Too many warnings for now nw="$nw -Wpadded" # Our structs are not padded nw="$nw -Wredundant-decls" # openat.h declares e.g., mkdirat nw="$nw -Wlogical-op" # any use of fwrite provokes this nw="$nw -Wformat-nonliteral" # who.c and pinky.c strftime uses nw="$nw -Wvla" # warnings in gettext.h nw="$nw -Wnested-externs" # use of XARGMATCH/verify_function__ nw="$nw -Wswitch-enum" # Too many warnings for now nw="$nw -Wswitch-default" # Too many warnings for now nw="$nw -Wstack-protector" # not worth working around # things I might fix soon: nw="$nw -Wfloat-equal" # sort.c, seq.c nw="$nw -Wmissing-format-attribute" # copy.c nw="$nw -Wunsafe-loop-optimizations" # a few src/*.c nw="$nw -Winline" # system.h's readdir_ignoring_dot_and_dotdot nw="$nw -Wstrict-overflow" # expr.c, pr.c, tr.c, factor.c # ?? -Wstrict-overflow gl_MANYWARN_ALL_GCC([ws]) gl_MANYWARN_COMPLEMENT([ws], [$ws], [$nw]) for w in $ws; do gl_WARN_ADD([$w]) done gl_WARN_ADD([-Wno-missing-field-initializers]) # We need this one gl_WARN_ADD([-Wno-sign-compare]) # Too many warnings for now gl_WARN_ADD([-Wno-pointer-sign]) # Too many warnings for now gl_WARN_ADD([-Wno-unused-parameter]) # Too many warnings for now # In spite of excluding -Wlogical-op above, it is enabled, as of # gcc 4.5.0 20090517, and it provokes warnings in cat.c, dd.c, truncate.c gl_WARN_ADD([-Wno-logical-op]) gl_WARN_ADD([-fdiagnostics-show-option]) AC_SUBST([WARN_CFLAGS]) AC_DEFINE([lint], [1], [Define to 1 if the compiler is checking for lint.]) AC_DEFINE([_FORTIFY_SOURCE], [2], [enable compile-time and run-time bounds-checking, and some warnings]) AC_DEFINE([GNULIB_PORTCHECK], [1], [enable some gnulib portability checks]) fi I am hoping to find time to do this for libguestfs.
On Mon, Aug 10, 2009 at 10:26:03AM +0100, Matthew Booth wrote:
On 10/08/09 10:21, Jim Meyering wrote:
Matthew Booth wrote:
On 10/08/09 09:57, Jim Meyering wrote:
Here are a few warning-removal changes for daemon/: ... What warnings are you turning on? I normally compile with -Wall, and I don't get warning for unused function arguments:
I started with these: -Wall -W -Wextra
Now I see where you're coming from. ACK.
Do you find it's generally useful to turn these on across the board? In my experience, until you get down to zero compiler warnings and turn on -Werror, they often go unnoticed. Are we likely to be able to get down to zero compiler warnings?
Yes, I think this is a good goal to aim for. Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://et.redhat.com/~rjones/virt-df/
On Mon, Aug 10, 2009 at 10:15:45AM +0100, Matthew Booth wrote:
In general, I would like to get down to no compiler warnings. The biggest culprit in daemon, though, is rpcgen generated stuff.
In libvirt we have some Perl scripts that attempt to rewrite the output of rpcgen-generated code to remove warnings. It's quite a bit of hassle though, and of course depends closely on the version of rpcgen (which is enforced in libvirt I think). Personally I'd prefer it if we could compile the rpcgen code with -Wno-unused -fno-strict-alias. However automake doesn't support providing CFLAGS for just a single file. Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into Xen guests. http://et.redhat.com/~rjones/virt-p2v
On Mon, Aug 10, 2009 at 11:09:01AM +0100, Richard W.M. Jones wrote:
On Mon, Aug 10, 2009 at 10:15:45AM +0100, Matthew Booth wrote:
In general, I would like to get down to no compiler warnings. The biggest culprit in daemon, though, is rpcgen generated stuff.
In libvirt we have some Perl scripts that attempt to rewrite the output of rpcgen-generated code to remove warnings.
It's quite a bit of hassle though, and of course depends closely on the version of rpcgen (which is enforced in libvirt I think).
Personally I'd prefer it if we could compile the rpcgen code with -Wno-unused -fno-strict-alias. However automake doesn't support providing CFLAGS for just a single file.
You could compile a libtool convenience library with just the rpcgen code, and then link that to the real library. That would let you set CFLAGS for that the mini rpcgen code, and the end result would be the same since the rpcgen mini lib would be statically linked into the main library during build Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
Daniel P. Berrange wrote:
On Mon, Aug 10, 2009 at 11:09:01AM +0100, Richard W.M. Jones wrote:
On Mon, Aug 10, 2009 at 10:15:45AM +0100, Matthew Booth wrote:
In general, I would like to get down to no compiler warnings. The biggest culprit in daemon, though, is rpcgen generated stuff.
In libvirt we have some Perl scripts that attempt to rewrite the output of rpcgen-generated code to remove warnings.
It's quite a bit of hassle though, and of course depends closely on the version of rpcgen (which is enforced in libvirt I think).
Personally I'd prefer it if we could compile the rpcgen code with -Wno-unused -fno-strict-alias. However automake doesn't support providing CFLAGS for just a single file.
You could compile a libtool convenience library with just the rpcgen code, and then link that to the real library. That would let you set CFLAGS for that the mini rpcgen code, and the end result would be the same since the rpcgen mini lib would be statically linked into the main library during build
Good idea.
On Mon, Aug 10, 2009 at 11:11:14AM +0100, Daniel P. Berrange wrote:
On Mon, Aug 10, 2009 at 11:09:01AM +0100, Richard W.M. Jones wrote:
On Mon, Aug 10, 2009 at 10:15:45AM +0100, Matthew Booth wrote:
In general, I would like to get down to no compiler warnings. The biggest culprit in daemon, though, is rpcgen generated stuff.
In libvirt we have some Perl scripts that attempt to rewrite the output of rpcgen-generated code to remove warnings.
It's quite a bit of hassle though, and of course depends closely on the version of rpcgen (which is enforced in libvirt I think).
Personally I'd prefer it if we could compile the rpcgen code with -Wno-unused -fno-strict-alias. However automake doesn't support providing CFLAGS for just a single file.
You could compile a libtool convenience library with just the rpcgen code, and then link that to the real library. That would let you set CFLAGS for that the mini rpcgen code, and the end result would be the same since the rpcgen mini lib would be statically linked into the main library during build
Sounds like a good idea. Can -fno-strict-aliasing be applied only to a single file? I would imagine the answer must be yes. Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones Read my programming blog: http://rwmj.wordpress.com Fedora now supports 75 OCaml packages (the OPEN alternative to F#) http://cocan.org/getting_started_with_ocaml_on_red_hat_and_fedora
Richard W.M. Jones wrote:
On Mon, Aug 10, 2009 at 10:15:45AM +0100, Matthew Booth wrote:
In general, I would like to get down to no compiler warnings. The biggest culprit in daemon, though, is rpcgen generated stuff.
In libvirt we have some Perl scripts that attempt to rewrite the output of rpcgen-generated code to remove warnings.
It's quite a bit of hassle though, and of course depends closely on the version of rpcgen (which is enforced in libvirt I think).
Personally I'd prefer it if we could compile the rpcgen code with -Wno-unused -fno-strict-alias. However automake doesn't support providing CFLAGS for just a single file.
One option is to add this to the generated file: #if (__GNUC__ == 4 && 3 <= __GNUC_MINOR__) || 4 < __GNUC__ # pragma GCC diagnostic ignored "-Wunused" #endif I don't know if that will work with strict aliases.
On Mon, Aug 10, 2009 at 10:57:13AM +0200, Jim Meyering wrote:
Here are a few warning-removal changes for daemon/: char * -do_debug (char *subcmd, char **argv) +do_debug (char *subcmd ATTRIBUTE_UNUSED, char **argv ATTRIBUTE_UNUSED) { #if ENABLE_DEBUG_COMMAND int argc, i;
Is this correct? Note that the parameters are only unused if ENABLE_DEBUG_COMMAND is not defined, but if this symbol is defined then they are used. I'm worried that gcc will try to optimize them or something. The rest of the patch is fine - ACK. Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones Read my programming blog: http://rwmj.wordpress.com Fedora now supports 75 OCaml packages (the OPEN alternative to F#) http://cocan.org/getting_started_with_ocaml_on_red_hat_and_fedora
Richard W.M. Jones wrote:
On Mon, Aug 10, 2009 at 10:57:13AM +0200, Jim Meyering wrote:
Here are a few warning-removal changes for daemon/: char * -do_debug (char *subcmd, char **argv) +do_debug (char *subcmd ATTRIBUTE_UNUSED, char **argv ATTRIBUTE_UNUSED) { #if ENABLE_DEBUG_COMMAND int argc, i;
Is this correct? Note that the parameters are only unused if ENABLE_DEBUG_COMMAND is not defined, but if this symbol is defined then they are used. I'm worried that gcc will try to optimize them or something.
The rest of the patch is fine - ACK.
Good catch. I'll adjust it.
Richard W.M. Jones wrote:
On Mon, Aug 10, 2009 at 10:57:13AM +0200, Jim Meyering wrote:
Here are a few warning-removal changes for daemon/: char * -do_debug (char *subcmd, char **argv) +do_debug (char *subcmd ATTRIBUTE_UNUSED, char **argv ATTRIBUTE_UNUSED) { #if ENABLE_DEBUG_COMMAND int argc, i;
Is this correct? Note that the parameters are only unused if ENABLE_DEBUG_COMMAND is not defined, but if this symbol is defined then they are used. I'm worried that gcc will try to optimize them or something.
How's this? I nearly added _ATTRIBUTE or ATTRIBUTE_ to the new name, but that's pretty darn long, and seemed like overkill, considering it's only used in this one file.
From 42e83f38b158d05388786a62c3bac80136ec64b9 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering@redhat.com> Date: Mon, 10 Aug 2009 12:17:00 +0200 Subject: [PATCH libguestfs] build: daemon/do_debug: parameters aren't always unused
* daemon/debug.c (MAYBE_UNUSED): Define. (do_debug): Mark parameters as unused only when they really are unused. Spotted by Richard Jones. SCALAR(0xdd8370) prefer sizeof *VAR sizeof TYPE (no semantic change) --- daemon/debug.c | 8 +++++++- 1 files changed, 7 insertions(+), 1 deletions(-) diff --git a/daemon/debug.c b/daemon/debug.c index 5b3b5fa..5083398 100644 --- a/daemon/debug.c +++ b/daemon/debug.c @@ -60,8 +60,14 @@ static struct cmd cmds[] = { }; #endif +#if ! ENABLE_DEBUG_COMMAND +# define MAYBE_UNUSED ATTRIBUTE_UNUSED +#else +# define MAYBE_UNUSED /* empty */ +#endif + char * -do_debug (char *subcmd ATTRIBUTE_UNUSED, char **argv ATTRIBUTE_UNUSED) +do_debug (char *subcmd MAYBE_UNUSED, char **argv MAYBE_UNUSED) { #if ENABLE_DEBUG_COMMAND int argc, i; -- 1.6.4.300.gc9df
On Mon, Aug 10, 2009 at 12:20:53PM +0200, Jim Meyering wrote:
How's this? I nearly added _ATTRIBUTE or ATTRIBUTE_ to the new name, but that's pretty darn long, and seemed like overkill, considering it's only used in this one file.
+#if ! ENABLE_DEBUG_COMMAND +# define MAYBE_UNUSED ATTRIBUTE_UNUSED +#else +# define MAYBE_UNUSED /* empty */ +#endif + char * -do_debug (char *subcmd ATTRIBUTE_UNUSED, char **argv ATTRIBUTE_UNUSED) +do_debug (char *subcmd MAYBE_UNUSED, char **argv MAYBE_UNUSED) { #if ENABLE_DEBUG_COMMAND int argc, i;
Looks good to me - ACK. Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones New in Fedora 11: Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 70 libraries supprt'd http://fedoraproject.org/wiki/MinGW http://www.annexia.org/fedora_mingw
participants (4)
-
Daniel P. Berrange -
Jim Meyering -
Matthew Booth -
Richard W.M. Jones