[Libosinfo] [libosinfo] Allow to use system pci.ids/usb.ids files

[Daniel P. Berrange]

On Thu, Feb 28, 2013 at 12:33:13PM +0100, Christophe Fergeau wrote:
> Most distros already ship copies of pci.ids/usb.ids. This commit
> allows to make use of these rather than the ones shipped with
> libosinfo. This is achieved through the use of
> --with-usb-ids-path and --with-pci-ids-path configure flags.
> ---
>  configure.ac           | 24 ++++++++++++++++++++++++
>  data/Makefile.am       | 25 ++++++++++++++-----------
>  libosinfo.spec.in      |  2 +-
>  osinfo/osinfo_loader.c | 45 +++++++++++++++++++++++++++++++++++++++++----
>  4 files changed, 80 insertions(+), 16 deletions(-)

> diff --git a/osinfo/osinfo_loader.c b/osinfo/osinfo_loader.c
> index 76e9bc2..ec8c94c 100644
> --- a/osinfo/osinfo_loader.c
> +++ b/osinfo/osinfo_loader.c
> @@ -1653,10 +1653,26 @@ osinfo_loader_process_file(OsinfoLoader *loader,
>      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);
> +        else {
> +            gchar *pci_ids_filename;
> +            gchar *usb_ids_filename;
> +#if defined(PCI_IDS)
> +            pci_ids_filename = g_path_get_basename(PCI_IDS);
> +#else
> +            pci_ids_filename = g_strdup("pci.ids");
> +#endif
> +#if defined(USB_IDS)
> +            usb_ids_filename = g_path_get_basename(USB_IDS);
> +#else
> +            usb_ids_filename = g_strdup("usb.ids");
> +#endif

This seems pretty strange to me - I don't see why we need to change this
part of the code at all in fact. If the user wants to drop a usb.ids
file into the osinfo database directory we should load that regardless
of whether we shipped one in the RPM or pointed at the common file
instead.

> +            if (strcmp(name, pci_ids_filename) == 0)
> +                osinfo_loader_process_file_reg_pci(loader, file, info, &error);
> +            else if (strcmp(name, usb_ids_filename) == 0)
> +                osinfo_loader_process_file_reg_usb(loader, file, info, &error);
> +            g_free(pci_ids_filename);
> +            g_free(usb_ids_filename);
> +        }
>          break;
>  
>      case G_FILE_TYPE_DIRECTORY:
> @@ -1782,6 +1798,27 @@ void osinfo_loader_process_system_path(OsinfoLoader *loader,
>                                 FALSE,
>                                 err);
>      g_object_unref(file);
> +    if (error_is_set(err))
> +        return;
> +
> +#ifdef PCI_IDS
> +    /* Load external pci.ids file */
> +    file = g_file_new_for_path(PCI_IDS);
> +    osinfo_loader_process_file(loader, file, FALSE, err);
> +    g_object_unref(file);
> +    if (error_is_set(err))
> +        return;
> +#endif
> +
> +#ifdef USB_IDS
> +    /* Load external usb.ids file */
> +    file = g_file_new_for_path(USB_IDS);
> +    osinfo_loader_process_file(loader, file, FALSE, err);
> +    g_object_unref(file);
> +    if (error_is_set(err))
> +        return;
> +#endif

I think rather than doing this, I'd just make /usr/share/osinfo/db/devices/pci.ids
be a symlink to the real shared file. Then we don't need any code changes
at all, both scenarios would work in the same way.

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|