Re: [PATCH v3 56/60] loader: rewrite loader to separate file enumeration from loading

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Fedora Users]     [Fedora Maintainers]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]

  Powered by Linux