Re: [libvirt] [PATCH] build: avoid shadowing devname() on BSD systems
On Fri, Dec 03, 2010 at 11:31:18AM -0700, Eric Blake wrote: On 12/03/2010 10:08 AM, Justin Clift wrote: Thanks Eric. Captured the complete make log this time, to ensure it's fixed. The same warning also appears in a few other places. :/ libvirt.c: In function 'virDomainOpenConsole': libvirt.c:13169: warning: declaration of 'devname' shadows a global declaration [-Wshadow] /usr/include/stdlib.h:290: warning: shadowed declaration is here [-Wshadow] Yuck. 'devname' is just too handy to be penalized by BSD's namespace pollution. What if we instead do this in internal.h: #include stdlib.h /* Silence -Wshadow on BSD systems, which declare a devname() that we * don't care about */ #define devname vir_devname then all other shadowing warnings should just go away, without us having to worry about the problem cropping up again. I dunno, I'd just change our variable to dname, or devicename. Regards, Daniel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] build: avoid shadowing devname() on BSD systems
On 12/06/2010 04:06 AM, Daniel P. Berrange wrote: libvirt.c: In function 'virDomainOpenConsole': libvirt.c:13169: warning: declaration of 'devname' shadows a global declaration [-Wshadow] /usr/include/stdlib.h:290: warning: shadowed declaration is here [-Wshadow] Yuck. 'devname' is just too handy to be penalized by BSD's namespace pollution. What if we instead do this in internal.h: #include stdlib.h /* Silence -Wshadow on BSD systems, which declare a devname() that we * don't care about */ #define devname vir_devname then all other shadowing warnings should just go away, without us having to worry about the problem cropping up again. I dunno, I'd just change our variable to dname, or devicename. The problem is that most development is done on glibc systems where devname is not a shadowing issue, so we risk re-introducing the problem for the next person who tries to build on BSD. Performing the rename via #define up front avoids the problem, while leaving the obvious variable name for easy use in the rest of code (then again, for gdb purposes, a #define rename should only be done when HAVE_DECL_DEVNAME, so as not to make debugging confusing on glibc systems). -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] build: avoid shadowing devname() on BSD systems
On 12/03/2010 10:48 AM, Justin Clift wrote: event.c: In function 'virEventInterruptLocked': event.c:656: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] VIR_DEBUG(Skip interrupt, %d %d, eventLoop.running, (int)eventLoop.leader.thread); return 0; } Like 656 is the VIR_DEBUG() line. Having trouble finding where eventLoop.leader.thread is defined though. Probably because I'm more sleepy than optimal. Thinking it might be some kind of problem with OSX and gnulib? Rather, it's due to the fact that pthread_t is allowed to be a pointer type, and on 64-bit systems, a pthread_t pointer is truncated when cast to int (it just happens that pthread_t is an integer rather than a pointer on glibc, so we don't notice this on Linux). But we already have virThreadSelfID which works around this issue for the current thread; all we need to do is extend it to other threads. Patch coming up soon! -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] build: avoid shadowing devname() on BSD systems
* tools/virsh.c (cmdRunConsole, cmdConsole): Rename problematic
symbol.
Reported by Justin Clift.
---
IIRC, it had the same warning on FreeBSD, I just didn't post a patch for it
yet.
Pushing under the build-breaker rule, then.
tools/virsh.c | 10 +-
1 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index 3e1bde1..441fd77 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -732,7 +732,7 @@ static const vshCmdOptDef opts_console[] = {
};
static int
-cmdRunConsole(vshControl *ctl, virDomainPtr dom, const char *devname)
+cmdRunConsole(vshControl *ctl, virDomainPtr dom, const char *name)
{
int ret = FALSE;
virDomainInfo dominfo;
@@ -749,7 +749,7 @@ cmdRunConsole(vshControl *ctl, virDomainPtr dom, const char
*devname)
vshPrintExtra(ctl, _(Connected to domain %s\n), virDomainGetName(dom));
vshPrintExtra(ctl, %s, _(Escape character is ^]\n));
-if (vshRunConsole(dom, devname) == 0)
+if (vshRunConsole(dom, name) == 0)
ret = TRUE;
cleanup:
@@ -762,7 +762,7 @@ cmdConsole(vshControl *ctl, const vshCmd *cmd)
{
virDomainPtr dom;
int ret;
-const char *devname;
+const char *name;
if (!vshConnectionUsability(ctl, ctl-conn))
return FALSE;
@@ -770,9 +770,9 @@ cmdConsole(vshControl *ctl, const vshCmd *cmd)
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
return FALSE;
-devname = vshCommandOptString(cmd, devname, NULL);
+name = vshCommandOptString(cmd, devname, NULL);
-ret = cmdRunConsole(ctl, dom, devname);
+ret = cmdRunConsole(ctl, dom, name);
virDomainFree(dom);
return ret;
--
1.7.3.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] build: avoid shadowing devname() on BSD systems
On 04/12/2010, at 2:30 AM, Eric Blake wrote: * tools/virsh.c (cmdRunConsole, cmdConsole): Rename problematic symbol. Reported by Justin Clift. --- IIRC, it had the same warning on FreeBSD, I just didn't post a patch for it yet. Pushing under the build-breaker rule, then. tools/virsh.c | 10 +- 1 files changed, 5 insertions(+), 5 deletions(-) Thanks Eric. Captured the complete make log this time, to ensure it's fixed. The same warning also appears in a few other places. :/ libvirt.c: In function 'virDomainOpenConsole': libvirt.c:13169: warning: declaration of 'devname' shadows a global declaration [-Wshadow] /usr/include/stdlib.h:290: warning: shadowed declaration is here [-Wshadow] remote/remote_driver.c: In function 'remoteDomainOpenConsole': remote/remote_driver.c:9250: warning: declaration of 'devname' shadows a global declaration [-Wshadow] /usr/include/stdlib.h:290: warning: shadowed declaration is here [-Wshadow] console.c: In function 'vshRunConsole': console.c:279: warning: declaration of 'devname' shadows a global declaration [-Wshadow] /usr/include/stdlib.h:290: warning: shadowed declaration is here [-Wshadow] libvirt.c: In function 'libvirt_virDomainOpenConsole': libvirt.c:1255: warning: declaration of 'devname' shadows a global declaration [-Wshadow] /usr/include/stdlib.h:290: warning: shadowed declaration is here [-Wshadow] While looking for those, a few more warnings also showed up: ../daemon/event.c: In function 'virEventInterruptLocked': ../daemon/event.c:656: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] virsh.c: In function 'vshReadlineInit': virsh.c:11575: warning: assignment discards qualifiers from pointer target type event.c: In function 'virEventInterruptLocked': event.c:656: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] Again though, they're warnings and don't stop the compilation from completing. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] build: avoid shadowing devname() on BSD systems
2010/12/3 Eric Blake ebl...@redhat.com: * tools/virsh.c (cmdRunConsole, cmdConsole): Rename problematic symbol. Reported by Justin Clift. --- IIRC, it had the same warning on FreeBSD, I just didn't post a patch for it yet. Pushing under the build-breaker rule, then. Actually on FreeBSD this shadowing problem doesn't affect virsh.c only, but libvirt itself too. For example, the virDomainOpenConsole function takes a devname argument. I should probably try to compile current git on FreeBSD again. Matthias -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] build: avoid shadowing devname() on BSD systems
Il giorno sab, 04/12/2010 alle 04.08 +1100, Justin Clift ha scritto: ../daemon/event.c: In function 'virEventInterruptLocked': ../daemon/event.c:656: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] event.c: In function 'virEventInterruptLocked': event.c:656: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] These two should be considered _more_ than simply warnings, they can abort the software at runtime if used on 64-bit systems, so I'd suggest tackling these _sooner_ than the shadows (I don't have compile-access to my mac right now, or I'd be sending something myself). -- Diego Elio Pettenò — “Flameeyes” http://blog.flameeyes.eu/ If you found a .asc file in this mail and know not what it is, it's a GnuPG digital signature: http://www.gnupg.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] build: avoid shadowing devname() on BSD systems
On 04/12/2010, at 4:16 AM, Diego Elio Pettenò wrote:
Il giorno sab, 04/12/2010 alle 04.08 +1100, Justin Clift ha scritto:
../daemon/event.c: In function 'virEventInterruptLocked':
../daemon/event.c:656: warning: cast from pointer to integer of
different size [-Wpointer-to-int-cast]
event.c: In function 'virEventInterruptLocked':
event.c:656: warning: cast from pointer to integer of different size
[-Wpointer-to-int-cast]
These two should be considered _more_ than simply warnings, they can
abort the software at runtime if used on 64-bit systems, so I'd suggest
tackling these _sooner_ than the shadows (I don't have compile-access to
my mac right now, or I'd be sending something myself).
Thanks. They're from this piece of code:
if (!eventLoop.running ||
virThreadIsSelf(eventLoop.leader)) {
VIR_DEBUG(Skip interrupt, %d %d, eventLoop.running,
(int)eventLoop.leader.thread);
return 0;
}
Like 656 is the VIR_DEBUG() line.
The eventLoop.running is defined earlier in the file as:
int running;
Having trouble finding where eventLoop.leader.thread is defined though.
Probably because
I'm more sleepy than optimal. Thinking it might be some kind of problem with
OSX and gnulib?
Tracing things back manually shows this in the OSX provided
/usr/include/pthread.h:
#ifndef _PTHREAD_T
#define _PTHREAD_T
typedef __darwin_pthread_t pthread_t;
#endif
And this in the gnulib version, gnulib/lib/pthread.h:
typedef int pthread_t;
Any idea if this on the right track, or am I just confusing myself? :)
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] build: avoid shadowing devname() on BSD systems
On 12/03/2010 10:08 AM, Justin Clift wrote: Thanks Eric. Captured the complete make log this time, to ensure it's fixed. The same warning also appears in a few other places. :/ libvirt.c: In function 'virDomainOpenConsole': libvirt.c:13169: warning: declaration of 'devname' shadows a global declaration [-Wshadow] /usr/include/stdlib.h:290: warning: shadowed declaration is here [-Wshadow] Yuck. 'devname' is just too handy to be penalized by BSD's namespace pollution. What if we instead do this in internal.h: #include stdlib.h /* Silence -Wshadow on BSD systems, which declare a devname() that we * don't care about */ #define devname vir_devname then all other shadowing warnings should just go away, without us having to worry about the problem cropping up again. (I've seen this trick used in gnulib, where we got rid of shadowing warnings for the poorly-named and now-obsolete index().) While looking for those, a few more warnings also showed up: ../daemon/event.c: In function 'virEventInterruptLocked': ../daemon/event.c:656: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] virsh.c: In function 'vshReadlineInit': virsh.c:11575: warning: assignment discards qualifiers from pointer target type event.c: In function 'virEventInterruptLocked': event.c:656: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] I agree with Diego - these are real bugs and need fixing. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
