Re: [Libvir] [patch 3/3] Do not inline xstrtol functions

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.

> > 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.