[Libguestfs] [PATCH v4 1/9] lib/osinfo.c: Extract xml processing into a callback

[Cedric Bosdonnat]

On Wed, 2017-03-08 at 18:45 +0100, Pino Toscano wrote:
> On Tuesday, 7 March 2017 15:26:57 CET 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.
> > ---
> 
> Mostly LGTM, just a couple of style issues.
> 
> >  lib/osinfo.c | 85 +++++++++++++++++++++++++++++++-----------------------------
> >  1 file changed, 44 insertions(+), 41 deletions(-)
> > 
> > diff --git a/lib/osinfo.c b/lib/osinfo.c
> > index ea2a7659a..121fac1ba 100644
> > --- a/lib/osinfo.c
> > +++ b/lib/osinfo.c
> > @@ -52,6 +52,7 @@
> >  #include <string.h>
> >  #include <unistd.h>
> >  #include <dirent.h>
> > +#include <errno.h>
> >  #include <assert.h>
> >  #include <sys/types.h>
> >  #include <libintl.h>
> > @@ -71,10 +72,14 @@ gl_lock_define_initialized (static, osinfo_db_lock);
> >  static ssize_t osinfo_db_size = 0; /* 0 = unread, -1 = error, >= 1 = #records */
> >  static struct osinfo *osinfo_db = NULL;
> >  
> > -static int read_osinfo_db (guestfs_h *g);
> > -static void free_osinfo_db_entry (struct osinfo *);
> > -
> >  #define XMLSTREQ(a,b) (xmlStrEqual((a),(b)) == 1)
> > +typedef int (*read_osinfo_db_callback) (guestfs_h *g, const char *path, void *opaque);
> > +
> > +static int
> > +read_osinfo_db (guestfs_h *g,
> > +                read_osinfo_db_callback callback, void *opaque);
> 
> Forward declarations in a single line please. (Actual implementations
> are fine as wrapped.)
> 
> > +static int read_osinfo_db_xml (guestfs_h *g, const char *pathname, void *data);
> 
> This forward declaration could stay where it was before (i.e. below).
> 
> > +static void free_osinfo_db_entry (struct osinfo *);
> >  
> >  /* Given one or more fields from the header of a CD/DVD/ISO, look up
> >   * the media in the libosinfo database and return our best guess for
> > @@ -87,14 +92,24 @@ static void free_osinfo_db_entry (struct osinfo *);
> >   */
> >  int
> >  guestfs_int_osinfo_map (guestfs_h *g, const struct guestfs_isoinfo *isoinfo,
> > -			const struct osinfo **osinfo_ret)
> > +                        const struct osinfo **osinfo_ret)
> 
> Extra indentation change.

No, just tabs replaced by spaces
All other changes are already made in my local copy.

--
Cedric

> >  {
> >    size_t i;
> >  
> >    /* We only need to lock the database when reading it for the first time. */
> >    gl_lock_lock (osinfo_db_lock);
> >    if (osinfo_db_size == 0) {
> > -    if (read_osinfo_db (g) == -1) {
> > +    if (read_osinfo_db (g, read_osinfo_db_xml, NULL) == -1) {
> > +      /* Fatal error: free any database entries which have been read, and
> > +       * mark the database as having a permanent error.
> > +       */
> > +      if (osinfo_db_size > 0) {
> > +        for (i = 0; i < (size_t) osinfo_db_size; ++i)
> > +          free_osinfo_db_entry (&osinfo_db[i]);
> > +      }
> > +      free (osinfo_db);
> > +      osinfo_db = NULL;
> > +      osinfo_db_size = -1;
> >        gl_lock_unlock (osinfo_db_lock);
> >        return -1;
> >      }
> > @@ -156,19 +171,18 @@ guestfs_int_osinfo_map (guestfs_h *g, const struct guestfs_isoinfo *isoinfo,
> >   * Try to use the shared osinfo database layout (and location) first:
> >   * https://gitlab.com/libosinfo/libosinfo/blob/master/docs/database-layout.txt
> >   */
> > -static int read_osinfo_db_xml (guestfs_h *g, const char *filename);
> > -
> > -static int read_osinfo_db_flat (guestfs_h *g, const char *directory);
> > -static int read_osinfo_db_three_levels (guestfs_h *g, const char *directory);
> > -static int read_osinfo_db_directory (guestfs_h *g, const char *directory);
> > +static int read_osinfo_db_flat (guestfs_h *g, const char *directory,
> > +                                read_osinfo_db_callback callback, void *opaque);
> > +static int read_osinfo_db_three_levels (guestfs_h *g, const char *directory,
> > +                                        read_osinfo_db_callback callback, void *opaque);
> > +static int read_osinfo_db_directory (guestfs_h *g, const char *directory,
> > +                                     read_osinfo_db_callback callback, void *opaque);
> 
> Ditto (wrapping).
> 
> Thanks,
> _______________________________________________
> Libguestfs mailing list
> Libguestfs at redhat.com
> https://www.redhat.com/mailman/listinfo/libguestfs