
On Thu, Feb 07, 2008 at 01:55:58PM +0000, Mark McLoughlin wrote:
On Thu, 2008-02-07 at 08:39 -0500, Daniel Veillard wrote:
On Thu, Feb 07, 2008 at 10:10:34AM +0000, Mark McLoughlin wrote:
On Thu, 2008-02-07 at 05:02 -0500, Daniel Veillard wrote:
On Wed, Feb 06, 2008 at 11:07:55PM +0000, Mark McLoughlin wrote:
Alternative is to not build with -Winline.
That sounds a weak way to try to avoid a problem, we should not rely on just compiler options to get the code to compile and link.
My preference would be to use the patch to make them real internal APIs without exporting all the functions, I think only xstrtol_i is used by external programs (virsh and qemud), and maybe we can add only that one to the list of exported symbols.
Yeah, good point. Like this?
Yup fine by me, maybe add a #include to allow using the function name without the __ within the library code, allowing for more uniform code.
Did you miss this:
#define virStrToLong_i(s,e,b,r) __virStrToLong_i((s),(e),(b),(r))
Or ...? Sorry, I'm not following.
Yes I missed it :-) , but I looked really !
Index: libvirt/src/nodeinfo.h =================================================================== --- libvirt.orig/src/nodeinfo.h 2008-02-07 10:05:32.000000000 +0000 +++ libvirt/src/nodeinfo.h 2008-02-07 10:05:46.000000000 +0000 @@ -24,7 +24,7 @@ #ifndef __VIR_NODEINFO_H__ #define __VIR_NODEINFO_H__
-#include "internal.h" +#include "libvirt/libvirt.h"
#ifdef __cplusplus extern "C" {
Seems a bit unrelated to the initial issue, but that's fine, I just didn't spot it the first time.
Yeah, it looks a bit bogus ... but it is related. internal.h was only needed for xstrtol() use in nodeinfo.c, so I would have replace it with util.h, except util.h is not needed by the header itself so I included the appropriate header in nodeinfo.h and util.h in nodeinfo.c.
okay, okay, +1 -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/