[libvirt] [PATCH] virCommand: Cleanup unnecessarily variables

--- Building with ENABLE_DEBUG=0 showed more errors, because VIR_DEBUG() get replaced with dummy 'do {} while(0)' statement, and compiler complains about unused parameters. So if anybody knows how to get rid of this ... Note that I am speaking about '...' part in VIR_DEBUG_INT() definition. src/util/command.c | 9 +++------ 1 files changed, 3 insertions(+), 6 deletions(-) diff --git a/src/util/command.c b/src/util/command.c index ebb90cb..4b8a940 100644 --- a/src/util/command.c +++ b/src/util/command.c @@ -962,9 +962,8 @@ virCommandProcessIO(virCommandPtr cmd) } else { inoff += done; if (inoff == inlen) { - int tmpfd = infd; if (VIR_CLOSE(infd) < 0) - VIR_DEBUG("ignoring failed close on fd %d", tmpfd); + VIR_DEBUG("ignoring failed close on fd %d", infd); } } } @@ -1145,17 +1144,15 @@ virCommandRun(virCommandPtr cmd, int *exitstatus) VIR_DEBUG("ignoring failed close on fd %d", tmpfd); } if (cmd->outbuf == &outbuf) { - int tmpfd = cmd->outfd; if (VIR_CLOSE(cmd->outfd) < 0) - VIR_DEBUG("ignoring failed close on fd %d", tmpfd); + VIR_DEBUG("ignoring failed close on fd %d", cmd->outfd); cmd->outfdptr = NULL; cmd->outbuf = NULL; VIR_FREE(outbuf); } if (cmd->errbuf == &errbuf) { - int tmpfd = cmd->errfd; if (VIR_CLOSE(cmd->errfd) < 0) - VIR_DEBUG("ignoring failed close on fd %d", tmpfd); + VIR_DEBUG("ignoring failed close on fd %d", cmd->errfd); cmd->errfdptr = NULL; cmd->errbuf = NULL; VIR_FREE(errbuf); -- 1.7.5.rc3

On 22.05.2011 16:42, Michal Privoznik wrote:
--- Building with ENABLE_DEBUG=0 showed more errors, because VIR_DEBUG() get replaced with dummy 'do {} while(0)' statement, and compiler complains about unused parameters. So if anybody knows how to get rid of this ... Note that I am speaking about '...' part in VIR_DEBUG_INT() definition.
src/util/command.c | 9 +++------ 1 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/src/util/command.c b/src/util/command.c index ebb90cb..4b8a940 100644 --- a/src/util/command.c +++ b/src/util/command.c @@ -962,9 +962,8 @@ virCommandProcessIO(virCommandPtr cmd) } else { inoff += done; if (inoff == inlen) { - int tmpfd = infd; if (VIR_CLOSE(infd) < 0) - VIR_DEBUG("ignoring failed close on fd %d", tmpfd); + VIR_DEBUG("ignoring failed close on fd %d", infd); } } } @@ -1145,17 +1144,15 @@ virCommandRun(virCommandPtr cmd, int *exitstatus) VIR_DEBUG("ignoring failed close on fd %d", tmpfd); } if (cmd->outbuf == &outbuf) { - int tmpfd = cmd->outfd; if (VIR_CLOSE(cmd->outfd) < 0) - VIR_DEBUG("ignoring failed close on fd %d", tmpfd); + VIR_DEBUG("ignoring failed close on fd %d", cmd->outfd); cmd->outfdptr = NULL; cmd->outbuf = NULL; VIR_FREE(outbuf); } if (cmd->errbuf == &errbuf) { - int tmpfd = cmd->errfd; if (VIR_CLOSE(cmd->errfd) < 0) - VIR_DEBUG("ignoring failed close on fd %d", tmpfd); + VIR_DEBUG("ignoring failed close on fd %d", cmd->errfd); cmd->errfdptr = NULL; cmd->errbuf = NULL; VIR_FREE(errbuf);
NACK. I misunderstood the concept of VIR_CLOSE. But the problem written above persists. Michal

On 05/22/2011 09:00 AM, Michal Prívozník wrote:
On 22.05.2011 16:42, Michal Privoznik wrote:
--- Building with ENABLE_DEBUG=0 showed more errors, because VIR_DEBUG() get replaced with dummy 'do {} while(0)' statement, and compiler complains about unused parameters. So if anybody knows how to get rid of this ... Note that I am speaking about '...' part in VIR_DEBUG_INT() definition.
if (inoff == inlen) { - int tmpfd = infd; if (VIR_CLOSE(infd) < 0) - VIR_DEBUG("ignoring failed close on fd %d", tmpfd); + VIR_DEBUG("ignoring failed close on fd %d", infd);
That won't work (infd is -1 at this point, not the value you meant to print). Either we don't care about the failure (and VIR_FORCE_CLOSE is the better fix), or we do care about it, and should be using something stronger than a VIR_DEBUG that can be compiled out. Using VIR_FORCE_CLOSE is probably the right fix for this immediate issue; another option would be marking the variable as (potentially) unused, as in: int tmpfd ATTRIBUTE_UNUSED; tmpfd = infd; But I can't help but wonder if the more generic issue could be solved by making VIR_DEBUG() (when ENABLE_DEBUG=0) actually call a static inline no-op varargs function, so that gcc treats it as using its arguments, rather than the current situation of expanding to nothing and leaving variables unused. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

On Sun, May 22, 2011 at 04:42:09PM +0200, Michal Privoznik wrote:
--- Building with ENABLE_DEBUG=0 showed more errors, because VIR_DEBUG() get replaced with dummy 'do {} while(0)' statement, and compiler complains about unused parameters. So if anybody knows how to get rid of this ... Note that I am speaking about '...' part in VIR_DEBUG_INT() definition.
src/util/command.c | 9 +++------ 1 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/src/util/command.c b/src/util/command.c index ebb90cb..4b8a940 100644 --- a/src/util/command.c +++ b/src/util/command.c @@ -962,9 +962,8 @@ virCommandProcessIO(virCommandPtr cmd) } else { inoff += done; if (inoff == inlen) { - int tmpfd = infd; if (VIR_CLOSE(infd) < 0) - VIR_DEBUG("ignoring failed close on fd %d", tmpfd); + VIR_DEBUG("ignoring failed close on fd %d", infd); } } } @@ -1145,17 +1144,15 @@ virCommandRun(virCommandPtr cmd, int *exitstatus) VIR_DEBUG("ignoring failed close on fd %d", tmpfd); } if (cmd->outbuf == &outbuf) { - int tmpfd = cmd->outfd; if (VIR_CLOSE(cmd->outfd) < 0) - VIR_DEBUG("ignoring failed close on fd %d", tmpfd); + VIR_DEBUG("ignoring failed close on fd %d", cmd->outfd); cmd->outfdptr = NULL; cmd->outbuf = NULL; VIR_FREE(outbuf); } if (cmd->errbuf == &errbuf) { - int tmpfd = cmd->errfd; if (VIR_CLOSE(cmd->errfd) < 0) - VIR_DEBUG("ignoring failed close on fd %d", tmpfd); + VIR_DEBUG("ignoring failed close on fd %d", cmd->errfd); cmd->errfdptr = NULL; cmd->errbuf = NULL; VIR_FREE(errbuf);
Anything which wants to ignore the return status of VIR_CLOSE and not report an error, should be using VIR_FORCE_CLOSE instead. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
participants (4)
-
Daniel P. Berrange
-
Eric Blake
-
Michal Privoznik
-
Michal Prívozník