Libguestfs - Feb 2017 - Re: [PATCH v3 04/10] lib/osinfo.c: Extract xml processing into a callback

On Fri, Feb 10, 2017 at 04:05:59PM +0100, Cédric Bosdonnat
wrote:
> In order to further reuse the osinfo database parsing in OCAML, this
> commit extracts the XML processing for the distro ISOs and places it
> into a newly created callback.
> 
> This will later help other code to traverse the osinfo DB files and
> let them extract what they need from them.
> diff --git a/lib/osinfo.c b/lib/osinfo.c
> index ea2a7659a..b77ff96b3 100644
> --- a/lib/osinfo.c
> +++ b/lib/osinfo.c
> @@ -43,6 +43,7 @@
>   *
>   * XXX Currently the database is not freed when the program exits /
>   * library is unloaded, although we should probably do that.
> + *
>   */
An extra line has been added to this comment.
> +#ifndef GUESTFS_PRIVATE
> +void guestfs_int_debug (guestfs_h *g, const char *fs, ...)
> +{
> +  va_list args;
> +
> +  va_start (args, fs);
> +  vfprintf (stderr, fs, args);
> +  va_end (args);
> +}
> +
> +void
> +guestfs_int_perrorf (guestfs_h *g, const char *fs, ...)
> +{
> +  va_list args;
> +  CLEANUP_FREE char *msg = NULL;
> +  int err;
> +
> +  va_start (args, fs);
> +  err = vasprintf (&msg, fs, args);
> +  va_end (args);
> +
> +  if (err < 0) return;
> +
> +  perror(msg);
> +}
> +#endif /* GUESTFS_PRIVATE */
Why have these functions been added in this commit?

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW

On Tue, 2017-02-14 at 15:35 +0000, Richard W.M. Jones
wrote:
> On Fri, Feb 10, 2017 at 04:05:59PM +0100, Cédric Bosdonnat wrote:
> > In order to further reuse the osinfo database parsing in OCAML, this
> > commit extracts the XML processing for the distro ISOs and places it
> > into a newly created callback.
> > 
> > This will later help other code to traverse the osinfo DB files and
> > let them extract what they need from them.
> > diff --git a/lib/osinfo.c b/lib/osinfo.c
> > index ea2a7659a..b77ff96b3 100644
> > --- a/lib/osinfo.c
> > +++ b/lib/osinfo.c
> > @@ -43,6 +43,7 @@
> >   *
> >   * XXX Currently the database is not freed when the program exits /
> >   * library is unloaded, although we should probably do that.
> > + *
> >   */
> 
> An extra line has been added to this comment.
Ooops
> > +#ifndef GUESTFS_PRIVATE
> > +void guestfs_int_debug (guestfs_h *g, const char *fs, ...)
> > +{
> > +  va_list args;
> > +
> > +  va_start (args, fs);
> > +  vfprintf (stderr, fs, args);
> > +  va_end (args);
> > +}
> > +
> > +void
> > +guestfs_int_perrorf (guestfs_h *g, const char *fs, ...)
> > +{
> > +  va_list args;
> > +  CLEANUP_FREE char *msg = NULL;
> > +  int err;
> > +
> > +  va_start (args, fs);
> > +  err = vasprintf (&msg, fs, args);
> > +  va_end (args);
> > +
> > +  if (err < 0) return;
> > +
> > +  perror(msg);
> > +}
> > +#endif /* GUESTFS_PRIVATE */
> 
> Why have these functions been added in this commit?
I could add them to the commit using the osinfo from mllib if you want.
Otherwise the functions are missing when linking the mllib ocaml / C wrapper.

--
Cedric

On Tue, Feb 14, 2017 at 05:19:27PM +0100, Cedric Bosdonnat
wrote:
> I could add them to the commit using the osinfo from mllib if you want.
> Otherwise the functions are missing when linking the mllib ocaml / C
wrapper.
That would make this commit simpler.

There could be a bigger problem though -- in theory the debug(),
error() etc macros should only be used from library code.  Your new
osinfo code is now being used as common code.  Now there are a few
ways to resolve this:

(1) Move debug(), error() etc to common/utils.  I'm not sure this will
actually work since they depend on g->verbose which is a private field
in the handle.

(2) Remove use of debug(), error() etc from the new mini-library.
Problem goes away completely if you can do this.

(3) Move the mini-library under common/osinfo (cf. common/visit for a
similar example).  Dependencies may make this difficult, but may also
point to problems.

Possibly (2) & (3) ...

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW