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

Daniel Veillard veillard at redhat.com
Thu Feb 7 14:33:53 UTC 2008


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 at redhat.com  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine  http://rpmfind.net/




More information about the libvir-list mailing list