Created attachment 196839 [details] [review]
Add object to get the vendor name from PNP IDs

Ported from gnome-settings-daemon's color plugin.

Patch is missing some gtk-doc love to get introspection working on it. Richard, could you handle that?

Created attachment 196977 [details] [review]
Add object to get the vendor name from PNP IDs

Ported from gnome-settings-daemon's color plugin.

Comment on attachment 196839 [details] [review]
Add object to get the vendor name from PNP IDs

New patch adds the necessary gtk-doc, and fixes some left-overs from the gnome-settings-daemon code.

Pushed to master.

Attachment 196977 [details] pushed as ec9da39 - Add object to get the vendor name from PNP IDs

Review of attachment 196977 [details] [review]:

Bastien, could you fix the following things:

::: libgnome-desktop/gnome-pnp-ids.c
@@ +30,3 @@
+struct _GnomePnpIdsPrivate
+{
+        gchar                           *data_dir;

data_dir is apparently useless, so let's drop it.

@@ +199,3 @@
+{
+        gboolean ret;
+        gchar *filename = NULL;

This variable is useless, drop it.

@@ +222,3 @@
+
+                /* the ID to text is a fixed offset */
+                retval[3] = '\0';

I'm nearly sure that the old code got fixed to not have the assumption that lines were well formatted. That's this line in the old code:
  if (line[0] && line[1] && line[2] && line[3] == '\t' && line[4])

If it got fixed, that's because it was needed :-) Can we have that back?

Hmmm, is this so that GnomeRR and g-s-d's color plugin can both lookup PNP stuff?  (In any case, it sounds good to have this available in such a clean little object - just wondering.)

Is there a reason for gnome_pnp_ids_new() working as a singleton?  It's not threadsafe, etc. - but I mean, the apps that need this are highly specialized, and they can be careful enough to only create a single object.  In any case, I can't imagine that reparsing the list of IDs would be a performance problem.

Vincent, all fixed.

commit 29a4d20f793cb30806e2d6209adf3696775dad48
Author: Bastien Nocera <hadess@hadess.net>
Date:   Tue Sep 20 11:15:27 2011 +0100

    gnome-pnp-ids: Remove unused filename variable

commit 3f748035c331ffeea2e3ac6e06b49ed1856404ed
Author: Bastien Nocera <hadess@hadess.net>
Date:   Tue Sep 20 11:12:05 2011 +0100

    gnome-pnp-ids: Remove unused data_dir variable

commit b7d00c429868c4c6e4b604ee4cf70fab369be869
Author: Bastien Nocera <hadess@hadess.net>
Date:   Tue Sep 20 11:10:02 2011 +0100

    gnome-pnp-ids: Re-add validation tweaks
    
    Which were lost in the port to the new object.

(In reply to comment #7)
> Hmmm, is this so that GnomeRR and g-s-d's color plugin can both lookup PNP
> stuff?  (In any case, it sounds good to have this available in such a clean
> little object - just wondering.)

Yep, that was the goal of the changes.

> Is there a reason for gnome_pnp_ids_new() working as a singleton?  It's not
> threadsafe, etc. - but I mean, the apps that need this are highly specialized,
> and they can be careful enough to only create a single object.  In any case, I
> can't imagine that reparsing the list of IDs would be a performance problem.

The problem is the fact that we were leaking that table *all the time* in gnome-desktop, and what's the point of re-parsing the file, and wasting memory when another part of the same process already has it parsed.

I'm happy to take patches to get the singleton be state of the art, rather than the current hack it is.