On Mon, Oct 12, 2015 at 06:11:17PM +0100, Daniel P. Berrange wrote:
> The current loader code enumerates files, processing them as it
> finds them. This changes it into a two stage process, first
> enumerating all files and then loading all files. This will
> facilitate later changes to way we enumerate files across
"to the way we enumerate" ? Current wording looks odd to me.
Seems fine otherwise.
Christophe
> multiple directories.
>
> Signed-off-by: Daniel P. Berrange <berrange@xxxxxxxxxx>
> ---
> osinfo/osinfo_loader.c | 152 +++++++++++++++++---------------------
> test/Makefile.am | 2 +-
> test/dbdata/oses/test-os-data.xml | 28 +++++++
> test/test-os.c | 2 +-
> test/test-os.xml | 31 --------
> 5 files changed, 99 insertions(+), 116 deletions(-)
> delete mode 100644 test/test-os.xml
>
> diff --git a/osinfo/osinfo_loader.c b/osinfo/osinfo_loader.c
> index b9e47ed..c516c92 100644
> --- a/osinfo/osinfo_loader.c
> +++ b/osinfo/osinfo_loader.c
> @@ -1,7 +1,7 @@
> /*
> * libosinfo:
> *
> - * Copyright (C) 2009-2012, 2014 Red Hat, Inc.
> + * Copyright (C) 2009-2015 Red Hat, Inc.
> *
> * This library is free software; you can redistribute it and/or
> * modify it under the terms of the GNU Lesser General Public
> @@ -1584,7 +1584,6 @@ static void osinfo_loader_process_xml(OsinfoLoader *loader,
> static void
> osinfo_loader_process_file_reg_ids(OsinfoLoader *loader,
> GFile *file,
> - GFileInfo *info,
> gboolean withSubsys,
> const char *baseURI,
> const char *busType,
> @@ -1710,12 +1709,10 @@ osinfo_loader_process_file_reg_ids(OsinfoLoader *loader,
> static void
> osinfo_loader_process_file_reg_usb(OsinfoLoader *loader,
> GFile *file,
> - GFileInfo *info,
> GError **err)
> {
> osinfo_loader_process_file_reg_ids(loader,
> file,
> - info,
> FALSE,
> "http://usb.org";,
> "usb",
> @@ -1725,12 +1722,10 @@ osinfo_loader_process_file_reg_usb(OsinfoLoader *loader,
> static void
> osinfo_loader_process_file_reg_pci(OsinfoLoader *loader,
> GFile *file,
> - GFileInfo *info,
> GError **err)
> {
> osinfo_loader_process_file_reg_ids(loader,
> file,
> - info,
> TRUE,
> "http://pcisig.com";,
> "pci",
> @@ -1738,15 +1733,8 @@ osinfo_loader_process_file_reg_pci(OsinfoLoader *loader,
> }
>
> static void
> -osinfo_loader_process_file(OsinfoLoader *loader,
> - GFile *file,
> - gboolean ignoreMissing,
> - GError **err);
> -
> -static void
> osinfo_loader_process_file_reg_xml(OsinfoLoader *loader,
> GFile *file,
> - GFileInfo *info,
> GError **err)
> {
> gchar *xml = NULL;
> @@ -1764,88 +1752,72 @@ osinfo_loader_process_file_reg_xml(OsinfoLoader *loader,
> g_free(xml);
> }
>
> -static void
> -osinfo_loader_process_file_dir(OsinfoLoader *loader,
> - GFile *file,
> - GFileInfo *info,
> - GError **err)
> -{
> - GFileEnumerator *ents = g_file_enumerate_children(file,
> - "standard::*",
> - G_FILE_QUERY_INFO_NONE,
> - NULL,
> - err);
> - if (error_is_set(err))
> - return;
> -
> - GFileInfo *child;
> - while ((child = g_file_enumerator_next_file(ents, NULL, err)) != NULL) {
> - const gchar *name = g_file_info_get_name(child);
> - GFile *ent = g_file_get_child(file, name);
> -
> - osinfo_loader_process_file(loader, ent, FALSE, err);
> -
> - g_object_unref(ent);
> - g_object_unref(child);
> -
> - if (error_is_set(err))
> - break;
> - }
> -
> - g_object_unref(ents);
> -}
>
> static void
> osinfo_loader_process_file(OsinfoLoader *loader,
> GFile *file,
> - gboolean ignoreMissing,
> GError **err)
> {
> - GError *error = NULL;
> - GFileInfo *info = g_file_query_info(file,
> - "standard::*",
> - G_FILE_QUERY_INFO_NONE,
> - NULL,
> - &error);
> - const char *name;
> + const gchar *name = g_file_get_basename(file);
> +
> + if (g_str_has_suffix(name, ".xml"))
> + osinfo_loader_process_file_reg_xml(loader, file, err);
> + else if (strcmp(name, "usb.ids") == 0)
> + osinfo_loader_process_file_reg_usb(loader, file, err);
> + else if (strcmp(name, "pci.ids") == 0)
> + osinfo_loader_process_file_reg_pci(loader, file, err);
> +}
> +
>
> +static GList *osinfo_loader_find_files(OsinfoLoader *loader,
> + GFile *file,
> + GError **err)
> +{
> + GError *error = NULL;
> + GFileInfo *child;
> + GList *files = NULL;
> + GFileEnumerator *ents = g_file_enumerate_children(file,
> + "standard::*",
> + G_FILE_QUERY_INFO_NONE,
> + NULL,
> + &error);
> if (error) {
> - if (ignoreMissing &&
> - (error->code == G_IO_ERROR_NOT_FOUND)) {
> + if (error->code == G_IO_ERROR_NOT_FOUND) {
> g_error_free(error);
> - return;
> + return NULL;
> }
> g_propagate_error(err, error);
> - return;
> + return NULL;
> }
>
> - name = g_file_info_get_name(info);
> -
> - GFileType type = g_file_info_get_attribute_uint32(info,
> - G_FILE_ATTRIBUTE_STANDARD_TYPE);
> -
> - switch (type) {
> - case G_FILE_TYPE_REGULAR:
> - if (g_str_has_suffix(name, ".xml"))
> - osinfo_loader_process_file_reg_xml(loader, file, info, &error);
> - else if (strcmp(name, "usb.ids") == 0)
> - osinfo_loader_process_file_reg_usb(loader, file, info, &error);
> - else if (strcmp(name, "pci.ids") == 0)
> - osinfo_loader_process_file_reg_pci(loader, file, info, &error);
> - break;
> -
> - case G_FILE_TYPE_DIRECTORY:
> - osinfo_loader_process_file_dir(loader, file, info, &error);
> - break;
> + while ((child = g_file_enumerator_next_file(ents, NULL, err)) != NULL) {
> + const gchar *name = g_file_info_get_name(child);
> + GFile *ent = g_file_get_child(file, name);
> + GFileType type = g_file_info_get_attribute_uint32(child,
> + G_FILE_ATTRIBUTE_STANDARD_TYPE);
> + if (type == G_FILE_TYPE_REGULAR) {
> + if (g_str_has_suffix(name, ".xml") ||
> + g_str_equal(name, "usb.ids") ||
> + g_str_equal(name, "pci.ids"))
> + files = g_list_append(files, g_object_ref(ent));
> + } else if (type == G_FILE_TYPE_DIRECTORY) {
> + GList *subfiles = osinfo_loader_find_files(loader, ent, &error);
> + if (subfiles) {
> + files = g_list_concat(files, subfiles);
> + }
> + }
> + g_object_unref(ent);
> + g_object_unref(child);
>
> - default:
> - break;
> + if (error) {
> + g_list_foreach(files, (GFunc)g_object_unref, NULL);
> + g_list_free(files);
> + return NULL;
> + }
> }
>
> - g_object_unref(info);
> -
> - if (error)
> - g_propagate_error(err, error);
> + g_object_unref(ents);
> + return files;
> }
>
>
> @@ -1858,15 +1830,29 @@ static void osinfo_loader_process_list(OsinfoLoader *loader,
> gpointer key, value;
>
> while (dirs && *dirs) {
> - osinfo_loader_process_file(loader,
> - *dirs,
> - TRUE,
> - &lerr);
> + GList *files = osinfo_loader_find_files(loader, *dirs, &lerr);
> + GList *tmp;
> if (lerr) {
> g_propagate_error(err, lerr);
> return;
> }
>
> + tmp = files;
> + while (tmp) {
> + osinfo_loader_process_file(loader, tmp->data, &lerr);
> + if (lerr) {
> + g_propagate_error(err, lerr);
> + break;
> + }
> + tmp = tmp->next;
> + }
> + g_list_foreach(files, (GFunc)g_object_unref, NULL);
> + g_list_free(files);
> +
> + if (lerr) {
> + break;
> + }
> +
> dirs++;
> }
>
> diff --git a/test/Makefile.am b/test/Makefile.am
> index 7c76861..5c05162 100644
> --- a/test/Makefile.am
> +++ b/test/Makefile.am
> @@ -1,5 +1,5 @@
>
> -EXTRA_DIST = isodata dbdata test-os.xml
> +EXTRA_DIST = isodata dbdata
>
> check_PROGRAMS = \
> test-entity \
> diff --git a/test/dbdata/oses/test-os-data.xml b/test/dbdata/oses/test-os-data.xml
> index 88ef431..1e6494e 100644
> --- a/test/dbdata/oses/test-os-data.xml
> +++ b/test/dbdata/oses/test-os-data.xml
> @@ -20,4 +20,32 @@
> <initrd>isolinux/initrd.img</initrd>
> </media>
> </os>
> +
> + <os id="http://libosinfo.org/test/os/test1";>
> + <short-id>test1</short-id>
> + <name>Test 1</name>
> + <version>unknown</version>
> + <vendor>libosinfo.org</vendor>
> + <family>test</family>
> + <release-status>prerelease</release-status>
> + </os>
> +
> + <os id="http://libosinfo.org/test/os/test2";>
> + <short-id>test2</short-id>
> + <release-status>released</release-status>
> + </os>
> +
> + <os id="http://libosinfo.org/test/os/test3";>
> + <short-id>test3</short-id>
> + </os>
> +
> + <os id="http://libosinfo.org/test/os/test4";>
> + <short-id>test4</short-id>
> + <release-status>snapshot</release-status>
> + </os>
> +
> + <os id="http://libosinfo.org/test/os/test5";>
> + <short-id>test5</short-id>
> + <release-status>invalid-value</release-status>
> + </os>
> </libosinfo>
> diff --git a/test/test-os.c b/test/test-os.c
> index 48ad8ac..8ee2403 100644
> --- a/test/test-os.c
> +++ b/test/test-os.c
> @@ -72,7 +72,7 @@ START_TEST(test_loader)
> const char *str;
>
> loader = osinfo_loader_new();
> - osinfo_loader_process_path(loader, SRCDIR "/test/test-os.xml", &error);
> + osinfo_loader_process_path(loader, SRCDIR "/test/dbdata", &error);
> fail_unless(error == NULL, error ? error->message:"none");
> db = osinfo_loader_get_db(loader);
>
> diff --git a/test/test-os.xml b/test/test-os.xml
> deleted file mode 100644
> index 883fe58..0000000
> --- a/test/test-os.xml
> +++ /dev/null
> @@ -1,31 +0,0 @@
> -<?xml version="1.0" encoding="UTF-8"?>
> -<libosinfo version="0.0.1">
> -
> - <os id="http://libosinfo.org/test/os/test1";>
> - <short-id>test1</short-id>
> - <name>Test 1</name>
> - <version>unknown</version>
> - <vendor>libosinfo.org</vendor>
> - <family>test</family>
> - <release-status>prerelease</release-status>
> - </os>
> -
> - <os id="http://libosinfo.org/test/os/test2";>
> - <short-id>test2</short-id>
> - <release-status>released</release-status>
> - </os>
> -
> - <os id="http://libosinfo.org/test/os/test3";>
> - <short-id>test3</short-id>
> - </os>
> -
> - <os id="http://libosinfo.org/test/os/test4";>
> - <short-id>test4</short-id>
> - <release-status>snapshot</release-status>
> - </os>
> -
> - <os id="http://libosinfo.org/test/os/test5";>
> - <short-id>test5</short-id>
> - <release-status>invalid-value</release-status>
> - </os>
> -</libosinfo>
> --
> 2.4.3
>
> _______________________________________________
> Libosinfo mailing list
> Libosinfo@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libosinfo
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Libosinfo mailing list Libosinfo@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libosinfo
