2:52 a.m.
Hi all,
Started seeing these warnings when compiling virsh:
virsh.c: In function 'cmdRunConsole':
virsh.c:735: warning: declaration of 'devname' shadows a global declaration
[-Wshadow]
/usr/include/stdlib.h:290: warning: shadowed declaration is here [-Wshadow]
virsh.c: In function 'cmdConsole':
virsh.c:765: warning: declaration of 'devname' shadows a global declaration
[-Wshadow]
/usr/include/stdlib.h:290: warning: shadowed declaration is here [-Wshadow]
virsh.c: In function 'vshReadlineInit':
virsh.c:11575: warning: assignment discards qualifiers from pointer target type
Haven't looked into it yet as they're only warnings. Anyone know if they're
important enough
to look into?
Regards and best wishes,
Justin Clift
3:40 a.m.
于 2010年12月03日 16:52, Justin Clift 写道:
Hi all,
Started seeing these warnings when compiling virsh:
virsh.c: In function 'cmdRunConsole':
virsh.c:735: warning: declaration of 'devname' shadows a global declaration
[-Wshadow]
/usr/include/stdlib.h:290: warning: shadowed declaration is here [-Wshadow]
virsh.c: In function 'cmdConsole':
virsh.c:765: warning: declaration of 'devname' shadows a global declaration
[-Wshadow]
/usr/include/stdlib.h:290: warning: shadowed declaration is here [-Wshadow]
virsh.c: In function 'vshReadlineInit':
virsh.c:11575: warning: assignment discards qualifiers from pointer target type
Haven't looked into it yet as they're only warnings. Anyone know if they're
important enough
to look into?
it works fine for me, even I set "./confugure
--enable-compile-warnings=maximum".
- Osier
3:47 a.m.
On 03/12/2010, at 8:40 PM, Osier Yang wrote:
于 2010年12月03日 16:52, Justin Clift 写道:
> Hi all,
>
> Started seeing these warnings when compiling virsh:
>
> virsh.c: In function 'cmdRunConsole':
> virsh.c:735: warning: declaration of 'devname' shadows a global declaration
[-Wshadow]
> /usr/include/stdlib.h:290: warning: shadowed declaration is here [-Wshadow]
> virsh.c: In function 'cmdConsole':
> virsh.c:765: warning: declaration of 'devname' shadows a global declaration
[-Wshadow]
> /usr/include/stdlib.h:290: warning: shadowed declaration is here [-Wshadow]
> virsh.c: In function 'vshReadlineInit':
> virsh.c:11575: warning: assignment discards qualifiers from pointer target type
>
> Haven't looked into it yet as they're only warnings. Anyone know if
they're important enough
> to look into?
>
it works fine for me, even I set "./confugure
--enable-compile-warnings=maximum".
Sorry, should have mentioned that they're only showing up in OSX, but not in Fedora
13. :)
4:04 a.m.
>> virsh.c: In function 'cmdRunConsole':
>> virsh.c:735: warning: declaration of 'devname' shadows a global
declaration [-Wshadow]
>> /usr/include/stdlib.h:290: warning: shadowed declaration is here [-Wshadow]
>> virsh.c: In function 'cmdConsole':
>> virsh.c:765: warning: declaration of 'devname' shadows a global
declaration [-Wshadow]
>> /usr/include/stdlib.h:290: warning: shadowed declaration is here [-Wshadow]
>> virsh.c: In function 'vshReadlineInit':
>> virsh.c:11575: warning: assignment discards qualifiers from pointer target
type
>>
>> Haven't looked into it yet as they're only warnings. Anyone know if
they're important enough
>> to look into?
Well, all warnings are important since we should be able to build with
--enable-compile-warnings=error
Sorry, should have mentioned that they're only showing up in OSX,
but not in Fedora 13. :)
Yeah, I was thinking it was the case. Apparently, OSX has devname declared in
/usr/include/stdlib.h:290. I wonder if it's only OSX or other BSD systems are
affected as well.
Jirka
4:55 a.m.
On 03/12/2010, at 9:04 PM, Jiri Denemark wrote:
>>> virsh.c: In function 'cmdRunConsole':
>>> virsh.c:735: warning: declaration of 'devname' shadows a global
declaration [-Wshadow]
>>> /usr/include/stdlib.h:290: warning: shadowed declaration is here [-Wshadow]
>>> virsh.c: In function 'cmdConsole':
>>> virsh.c:765: warning: declaration of 'devname' shadows a global
declaration [-Wshadow]
>>> /usr/include/stdlib.h:290: warning: shadowed declaration is here [-Wshadow]
>>> virsh.c: In function 'vshReadlineInit':
>>> virsh.c:11575: warning: assignment discards qualifiers from pointer target
type
>>>
>>> Haven't looked into it yet as they're only warnings. Anyone know if
they're important enough
>>> to look into?
Well, all warnings are important since we should be able to build with
--enable-compile-warnings=error
> Sorry, should have mentioned that they're only showing up in OSX, but not in
Fedora 13. :)
Yeah, I was thinking it was the case. Apparently, OSX has devname declared in
/usr/include/stdlib.h:290. I wonder if it's only OSX or other BSD systems are
affected as well.
Looks like it from what Matthias just mentioned. Looking in /usr/include/stdlib.h on the
box here,
it shows these:
int daemon(int, int) __DARWIN_1050(daemon)
__OSX_AVAILABLE_BUT_DEPRECATED(__MAC_10_0,__MAC_10_5,__IPHONE_2_0,__IPHONE_2_0);
char *devname(dev_t, mode_t);
char *devname_r(dev_t, mode_t, char *buf, int len);
char *getbsize(int *, long *);
int getloadavg(double [], int);
const char
*getprogname(void);
(included a few extra lines for context)
4:23 a.m.
2010/12/3 Justin Clift <jclift(a)redhat.com>:
On 03/12/2010, at 8:40 PM, Osier Yang wrote:
> 于 2010年12月03日 16:52, Justin Clift 写道:
>> Hi all,
>>
>> Started seeing these warnings when compiling virsh:
>>
>> virsh.c: In function 'cmdRunConsole':
>> virsh.c:735: warning: declaration of 'devname' shadows a global
declaration [-Wshadow]
>> /usr/include/stdlib.h:290: warning: shadowed declaration is here [-Wshadow]
>> virsh.c: In function 'cmdConsole':
>> virsh.c:765: warning: declaration of 'devname' shadows a global
declaration [-Wshadow]
>> /usr/include/stdlib.h:290: warning: shadowed declaration is here [-Wshadow]
>> virsh.c: In function 'vshReadlineInit':
>> virsh.c:11575: warning: assignment discards qualifiers from pointer target
type
>>
>> Haven't looked into it yet as they're only warnings. Anyone know if
they're important enough
>> to look into?
>>
> it works fine for me, even I set "./confugure
--enable-compile-warnings=maximum".
Sorry, should have mentioned that they're only showing up in OSX, but not in Fedora
13. :)
IIRC, it had the same warning on FreeBSD, I just didn't post a patch for it yet.
Matthias
9:30 a.m.
New subject: [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
11:08 a.m.
New subject: [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.
11:16 a.m.
New subject: [libvirt] [PATCH] build: avoid shadowing devname() on BSD systems
Il giorno sab, 04/12/2010 alle 04.08 +1100, Justin Clift ha scritto:
--
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/
../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).
11:48 a.m.
New subject: [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? :)
2:51 p.m.
New subject: [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 eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
3:33 p.m.
New subject: [libvirt] [PATCH] threads: add virThreadID for debugging use
* src/util/threads.h (virThreadID): New prototype.
* src/util/threads-pthread.c (virThreadID): New function.
* src/util/threads-win32.c (virThreadID): Likewise.
* src/libvirt_private.syms (threads.h): Export it.
* daemon/event.c (virEventInterruptLocked): Use it to avoid
warning on BSD systems.
---
daemon/event.c | 3 ++-
src/libvirt_private.syms | 1 +
src/util/threads-pthread.c | 6 ++++++
src/util/threads-win32.c | 5 +++++
src/util/threads.h | 5 +++++
5 files changed, 19 insertions(+), 1 deletions(-)
diff --git a/daemon/event.c b/daemon/event.c
index 0920748..01cb674 100644
--- a/daemon/event.c
+++ b/daemon/event.c
@@ -653,7 +653,8 @@ static int virEventInterruptLocked(void)
if (!eventLoop.running ||
virThreadIsSelf(&eventLoop.leader)) {
- VIR_DEBUG("Skip interrupt, %d %d", eventLoop.running,
(int)eventLoop.leader.thread);
+ VIR_DEBUG("Skip interrupt, %d %d", eventLoop.running,
+ virThreadID(&eventLoop.leader));
return 0;
}
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index aaf48ab..e2def6c 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -773,6 +773,7 @@ virMutexInitRecursive;
virMutexLock;
virMutexUnlock;
virThreadCreate;
+virThreadID;
virThreadIsSelf;
virThreadJoin;
virThreadSelf;
diff --git a/src/util/threads-pthread.c b/src/util/threads-pthread.c
index bff4979..826b9d1 100644
--- a/src/util/threads-pthread.c
+++ b/src/util/threads-pthread.c
@@ -22,6 +22,7 @@
#include <config.h>
#include <unistd.h>
+#include <inttypes.h>
#if HAVE_SYS_SYSCALL_H
# include <sys/syscall.h>
#endif
@@ -197,6 +198,11 @@ int virThreadSelfID(void)
#endif
}
+int virThreadID(virThreadPtr thread)
+{
+ return (int)(uintptr_t)thread->thread;
+}
+
void virThreadJoin(virThreadPtr thread)
{
pthread_join(thread->thread, NULL);
diff --git a/src/util/threads-win32.c b/src/util/threads-win32.c
index 436b3bd..2f4bcfc 100644
--- a/src/util/threads-win32.c
+++ b/src/util/threads-win32.c
@@ -304,6 +304,11 @@ int virThreadSelfID(void)
return (int)GetCurrentThreadId();
}
+int virThreadID(virThreadPtr thread)
+{
+ return (int)thread->thread;
+}
+
void virThreadJoin(virThreadPtr thread)
{
diff --git a/src/util/threads.h b/src/util/threads.h
index fa00a91..de28db6 100644
--- a/src/util/threads.h
+++ b/src/util/threads.h
@@ -51,7 +51,12 @@ int virThreadCreate(virThreadPtr thread,
void virThreadSelf(virThreadPtr thread);
bool virThreadIsSelf(virThreadPtr thread);
void virThreadJoin(virThreadPtr thread);
+
+/* These next two functions are for debugging only, since they are not
+ * guaranteed to give unique values for distinct threads on all
+ * architectures. */
int virThreadSelfID(void);
+int virThreadID(virThreadPtr thread);
int virMutexInit(virMutexPtr m) ATTRIBUTE_RETURN_CHECK;
int virMutexInitRecursive(virMutexPtr m) ATTRIBUTE_RETURN_CHECK;
--
1.7.3.2
10:10 a.m.
New subject: [libvirt] [PATCH] threads: add virThreadID for debugging use
On 05/12/2010, at 8:33 AM, Eric Blake wrote:
* src/util/threads.h (virThreadID): New prototype.
* src/util/threads-pthread.c (virThreadID): New function.
* src/util/threads-win32.c (virThreadID): Likewise.
* src/libvirt_private.syms (threads.h): Export it.
* daemon/event.c (virEventInterruptLocked): Use it to avoid
warning on BSD systems.
---
daemon/event.c | 3 ++-
src/libvirt_private.syms | 1 +
src/util/threads-pthread.c | 6 ++++++
src/util/threads-win32.c | 5 +++++
src/util/threads.h | 5 +++++
5 files changed, 19 insertions(+), 1 deletions(-)
As a data point, this patch fixes the "cast from pointer to integer
of different size [-Wpointer-to-int-cast]" warnings.
But it's beyond my personal coding level for reviewing. :/
12:58 p.m.
New subject: [libvirt] [PATCH] threads: add virThreadID for debugging use
2010/12/4 Eric Blake <eblake(a)redhat.com>:
* src/util/threads.h (virThreadID): New prototype.
* src/util/threads-pthread.c (virThreadID): New function.
* src/util/threads-win32.c (virThreadID): Likewise.
* src/libvirt_private.syms (threads.h): Export it.
* daemon/event.c (virEventInterruptLocked): Use it to avoid
warning on BSD systems.
---
diff --git a/src/util/threads-pthread.c b/src/util/threads-pthread.c
index bff4979..826b9d1 100644
--- a/src/util/threads-pthread.c
+++ b/src/util/threads-pthread.c
@@ -197,6 +198,11 @@ int virThreadSelfID(void)
#endif
}
+int virThreadID(virThreadPtr thread)
+{
+ return (int)(uintptr_t)thread->thread;
+}
+
void virThreadJoin(virThreadPtr thread)
{
pthread_join(thread->thread, NULL);
diff --git a/src/util/threads-win32.c b/src/util/threads-win32.c
index 436b3bd..2f4bcfc 100644
--- a/src/util/threads-win32.c
+++ b/src/util/threads-win32.c
@@ -304,6 +304,11 @@ int virThreadSelfID(void)
return (int)GetCurrentThreadId();
}
+int virThreadID(virThreadPtr thread)
+{
+ return (int)thread->thread;
+}
+
void virThreadJoin(virThreadPtr thread)
{
diff --git a/src/util/threads.h b/src/util/threads.h
index fa00a91..de28db6 100644
--- a/src/util/threads.h
+++ b/src/util/threads.h
@@ -51,7 +51,12 @@ int virThreadCreate(virThreadPtr thread,
void virThreadSelf(virThreadPtr thread);
bool virThreadIsSelf(virThreadPtr thread);
void virThreadJoin(virThreadPtr thread);
+
+/* These next two functions are for debugging only, since they are not
+ * guaranteed to give unique values for distinct threads on all
+ * architectures. */
int virThreadSelfID(void);
+int virThreadID(virThreadPtr thread);
This solve the problem, but it should also be noted that
virThreadSelfID and virThreadID cannot be used for comparison as they
are not taken from the same source.
ACK.
Matthias
10:50 a.m.
New subject: [libvirt] [PATCH] threads: add virThreadID for debugging use
On 12/05/2010 11:58 AM, Matthias Bolte wrote:
2010/12/4 Eric Blake <eblake(a)redhat.com>:
> * src/util/threads.h (virThreadID): New prototype.
> * src/util/threads-pthread.c (virThreadID): New function.
> * src/util/threads-win32.c (virThreadID): Likewise.
> * src/libvirt_private.syms (threads.h): Export it.
> * daemon/event.c (virEventInterruptLocked): Use it to avoid
> warning on BSD systems.
> ---
> +
> +/* These next two functions are for debugging only, since they are not
> + * guaranteed to give unique values for distinct threads on all
> + * architectures. */
> int virThreadSelfID(void);
> +int virThreadID(virThreadPtr thread);
>
This solve the problem, but it should also be noted that
virThreadSelfID and virThreadID cannot be used for comparison as they
are not taken from the same source.
ACK.
Thanks for the review; I'm updating the comment to this, then pushing:
/* These next two functions are for debugging only, since they are not
* guaranteed to give unique values for distinct threads on all
* architectures, nor are the two functions guaranteed to give the same
* value for the same thread. */
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
12:31 p.m.
New subject: [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 eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
5:06 a.m.
New subject: [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
10:30 a.m.
New subject: [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 eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
11:11 a.m.
New subject: [libvirt] [PATCH] build: avoid shadowing devname() on BSD systems
2010/12/3 Eric Blake <eblake(a)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
5100
days inactive
5103
days old
18 comments
7 participants
participants (7)
-
Daniel P. Berrange -
Diego Elio Pettenò -
Eric Blake -
Jiri Denemark -
Justin Clift -
Matthias Bolte -
Osier Yang