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.
{ 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, -- Pino Toscano