[PATCH] Add compressed ID database support to (usb|pci)-db

[PATCH] Add compressed ID database support to (usb|pci)-db

[Piter PUNK]

[-- Attachment #1: Type: text/plain, Size: 490 bytes --]

Hi

usbutils and pciutils have support to gzip'ed ID databases. But, if I 
use usb.ids.gz
or pci.ids.gz, usb-db and pci-db can't  show anything, giving only an 
error of
file not found. Looking in the code, they didn't have any support to 
compressed
ID files.

This patch add support to gzip'ed database files to usb-db.c and add the 
option
--disable-zlib to configure.ac. Most of code here come from usbutils names.c
file with little modifications to work with udev.

Bye,

Piter PUNK



[-- Attachment #2: udev-zlib-support.patch --]
[-- Type: text/plain, Size: 3111 bytes --]

diff --git a/configure.ac b/configure.ac
index 3afb94a..5a273e6 100644
--- a/configure.ac
+++ b/configure.ac
@@ -6,7 +6,7 @@ AC_DISABLE_STATIC
 AC_USE_SYSTEM_EXTENSIONS
 AC_SYS_LARGEFILE
 AC_CONFIG_MACRO_DIR([m4])
-LT_INIT
+m4_ifdef([LT_INIT],[LT_INIT],[AC_PROG_LIBTOOL])
 AC_PROG_AWK
 GTK_DOC_CHECK(1.10)
 AC_PREFIX_DEFAULT([/usr])
@@ -69,6 +69,22 @@ if test "x$enable_extras" = xyes; then
 	PKG_CHECK_MODULES(USBUTILS, usbutils >= 0.82)
 	AC_SUBST([USB_DATABASE], [$($PKG_CONFIG --variable=usbids usbutils)])
 
+	USE_ZLIB=yes
+	AC_ARG_ENABLE(zlib,
+		AS_HELP_STRING(--disable-zlib,disable support for zlib),
+		[
+			if eval "test x$enable_zlib = xno"; then
+				USE_ZLIB=
+			fi
+		])
+	if test "$USE_ZLIB" = "yes" ; then
+		AC_CHECK_LIB(z, inflateEnd)
+		if test "${ac_cv_lib_z_inflateEnd}" = "yes" ; then
+			HAVE_ZLIB="yes"
+		fi
+	fi
+	AM_CONDITIONAL(HAVE_ZLIB, test x$HAVE_ZLIB = xyes)
+
 	AC_CHECK_FILES([/usr/share/pci.ids], [pciids=/usr/share/pci.ids])
 	AC_CHECK_FILES([/usr/share/hwdata/pci.ids], [pciids=/usr/share/hwdata/pci.ids])
 	AC_CHECK_FILES([/usr/share/misc/pci.ids], [pciids=/usr/share/misc/pci.ids])
diff --git a/extras/usb-db/usb-db.c b/extras/usb-db/usb-db.c
index db1f843..1381c3c 100644
--- a/extras/usb-db/usb-db.c
+++ b/extras/usb-db/usb-db.c
@@ -39,6 +39,19 @@
 # error "Are you havin' a laugh?"
 #endif
 
+#ifdef HAVE_LIBZ
+#include <zlib.h>
+#define         data_file                        gzFile
+#define         data_fopen(path, mode)           gzopen(path, mode)
+#define         data_fgets(s, size, stream)      gzgets(stream, s, size)
+#define         data_close(f)                    gzclose(f)
+#else
+#define         data_file                        FILE*
+#define         data_fopen(path, mode)           fopen(path, mode)
+#define         data_fgets(s, size, stream)      fgets(s, size, stream)
+#define         data_close(f)                    fclose(f)
+#endif
+
 static int get_id_attr(
 	struct udev_device *parent,
 	const char *name,
@@ -98,29 +111,36 @@ static int lookup_vid_pid(
 	char **vendor,
 	char **product) {
 
-	FILE *f;
+	data_file f;
 	int ret = -1;
 	int found_vendor = 0;
-	char *line = NULL;
+	char buf[512], *line;
+	char *filename = DATABASE ;
 
 	*vendor = *product = NULL;
 
-	if (!(f = fopen(DATABASE, "r"))) {
+#ifdef HAVE_LIBZ
+	filename = DATABASE".gz";
+	if (!(f = data_fopen(filename, "r"))) {
+		filename = DATABASE;
+		if (!(f = data_fopen(filename, "r"))) {
+			fprintf(stderr, "Failed to open database file "DATABASE": %s\n", strerror(errno));
+			return -1;
+		}
+	}		
+#else
+	if (!(f = data_fopen(filename, "r"))) {
 		fprintf(stderr, "Failed to open database file "DATABASE": %s\n", strerror(errno));
 		return -1;
 	}
+#endif
 
 	for (;;) {
 		size_t n;
 
-		if (line) {
-			free(line);
-			line = NULL;
-		}
-
-		if (getline(&line, &n, f) < 0)
+		if (data_fgets(buf, sizeof(buf), f) < 0)
 			break;
-
+		line = buf;
 		rstrip(line);
 
 		if (line[0] == '#' || line[0] == 0)
@@ -171,8 +191,7 @@ static int lookup_vid_pid(
 	ret = 0;
 
 finish:
-	free(line);
-	fclose(f);
+	data_close(f);
 
 	if (ret < 0) {
 		free(*product);

Re: [PATCH] Add compressed ID database support to (usb|pci)-db

[Ozan Çağlayan]

Piter PUNK wrote:
> Hi
>
> This patch add support to gzip'ed database files to usb-db.c and add
> the option
> --disable-zlib to configure.ac. Most of code here come from usbutils
> names.c
> file with little modifications to work with udev.

Looking at the code it seems that enabling zlib support sets HAVE_ZLIB
which assumes that the DBs are always compressed with gzip which removes
the support for uncompressed db files. Logically, both should be
supported with some kind of file format detection.

Ignore this if I wrongly understand the intention :)

Regards,

Re: [PATCH] Add compressed ID database support to (usb|pci)-db

[Piter PUNK]

Ozan Çağlayan wrote:
> Piter PUNK wrote:
>   
>> This patch add support to gzip'ed database files to usb-db.c and add
>> the option
>> --disable-zlib to configure.ac. Most of code here come from usbutils
>> names.c
>> file with little modifications to work with udev.
>>     
> Looking at the code it seems that enabling zlib support sets HAVE_ZLIB
> which assumes that the DBs are always compressed with gzip which removes
> the support for uncompressed db files. Logically, both should be
> supported with some kind of file format detection.
>   

zlib functions can read compressed and uncompressed files. That's why I 
try to
open DATABASE.gz first and, if failed, open DATABASE when zlib support is
enabled. The support of both file formats is OK when zlib is enabled.

When disabled, only uncompressed files are supported. Almost no changes
there. The only drawback of enable zlib support is the need of zlib shared
library. But, who are using gzip'ed ID db files already needs zlib to have
working lsusb and lspci.

> Ignore this if I wrongly understand the intention :)
>   

Oh, i think is better to explain. Maybe someone else have the same doubt.

Hope it's all OK now -:)

Piter Punk

Re: [PATCH] Add compressed ID database support to (usb|pci)-db

[Lennart Poettering]

On Mon, 21.09.09 23:42, Piter PUNK (piterpunk@unitednerds.org) wrote:

> Hi

Heya,

> usbutils and pciutils have support to gzip'ed ID databases. But, if
> I use usb.ids.gz or pci.ids.gz, usb-db and pci-db can't show
> anything, giving only an error of file not found. Looking in the
> code, they didn't have any support to compressed ID files.
> 
> This patch add support to gzip'ed database files to usb-db.c and add
> the option --disable-zlib to configure.ac. Most of code here come
> from usbutils names.c file with little modifications to work with
> udev.

Patch looks mstly good. However after discussing this a little with
Kay we'd prefer if we could do without the macro orgy and just depend
unconditionally on zlib. Given that this is in extras this should not
be a problem.

That said, generally speaking it's bad enough that we do a linar
search for the vid/pid. Compressing things won't make things faster.

Is there actually a distribution that uses a compressed pci.ids file?

> diff --git a/configure.ac b/configure.ac
> index 3afb94a..5a273e6 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -6,7 +6,7 @@ AC_DISABLE_STATIC
>  AC_USE_SYSTEM_EXTENSIONS
>  AC_SYS_LARGEFILE
>  AC_CONFIG_MACRO_DIR([m4])
> -LT_INIT
> +m4_ifdef([LT_INIT],[LT_INIT],[AC_PROG_LIBTOOL])

Uh, this shouldn't be sneaked in here.

Lennart

-- 
Lennart Poettering                        Red Hat, Inc.
lennart [at] poettering [dot] net
http://0pointer.net/lennart/           GnuPG 0x1A015CC4

Re: [PATCH] Add compressed ID database support to (usb|pci)-db

[David Zeuthen]

On Wed, 2009-09-30 at 00:48 +0200, Lennart Poettering wrote:
> That said, generally speaking it's bad enough that we do a linar
> search for the vid/pid. 

You could do a binary search kind of deal, the entries are sorted. Not
sure it's worth the effort due to how things are laid out - you'd need
to search back or forward to find the first line not starting with
white-space.

> Compressing things won't make things faster.
> 
> Is there actually a distribution that uses a compressed pci.ids file?

None of the major distros does this kind of thing. And that is a good
thing - see below.

FWIW, this thing came up a couple of years ago on the hal list and back
then I very strongly refused to support compressed ids files. Mostly on
the grounds that it makes it impossible to mmap the files. E.g. you need
to allocate memory for the uncompressed image.

In udev this is even worse because we'd be doing all this work over and
over again - for every freaking event. In HAL, we mmaped the ids files
at start-up, built a simple look-up table and did binary lookups on
every event.

Just so you know, I'm still very much against this and I think it would
be a mistake to support compressed ids files in in udev. Please don't do
it.

      David

Re: [PATCH] Add compressed ID database support to (usb|pci)-db

[Lennart Poettering]

On Tue, 29.09.09 19:04, David Zeuthen (david@fubar.dk) wrote:

> FWIW, this thing came up a couple of years ago on the hal list and back
> then I very strongly refused to support compressed ids files. Mostly on
> the grounds that it makes it impossible to mmap the files. E.g. you need
> to allocate memory for the uncompressed image.
> 
> In udev this is even worse because we'd be doing all this work over and
> over again - for every freaking event. In HAL, we mmaped the ids files
> at start-up, built a simple look-up table and did binary lookups on
> every event.
> 
> Just so you know, I'm still very much against this and I think it would
> be a mistake to support compressed ids files in in udev. Please don't do
> it.

Ok, that makes sense to me. Sorry, Piter, it is unlikely that we will
merge your patch.

Even though your work will not be merged it is really appreciated!

Sorry,

Lennart

-- 
Lennart Poettering                        Red Hat, Inc.
lennart [at] poettering [dot] net
http://0pointer.net/lennart/           GnuPG 0x1A015CC4

Re: [PATCH] Add compressed ID database support to (usb|pci)-db

[Piter PUNK]

On Wed, 30 Sep 2009, Lennart Poettering wrote:
> Patch looks mstly good. However after discussing this a little with
> Kay we'd prefer if we could do without the macro orgy and just depend
> unconditionally on zlib. Given that this is in extras this should not
> be a problem.

The macro orgy is from usb-utils code -;)

> Is there actually a distribution that uses a compressed pci.ids file?

Slackware uses it on installer but the udev use there is very restrict.

After installed, the distribution uses uncompressed pci.ids and usb.ids.
I am doing some tests with usb-utils and pciutils packages and got the
errors from usb-db and pci-db. Because that i wrote the patch.

>> diff --git a/configure.ac b/configure.ac
>> index 3afb94a..5a273e6 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -6,7 +6,7 @@ AC_DISABLE_STATIC
>>  AC_USE_SYSTEM_EXTENSIONS
>>  AC_SYS_LARGEFILE
>>  AC_CONFIG_MACRO_DIR([m4])
>> -LT_INIT
>> +m4_ifdef([LT_INIT],[LT_INIT],[AC_PROG_LIBTOOL])
>
> Uh, this shouldn't be sneaked in here.

Sorry! I put this here to compile udev from git on Slackware.
Forgot the change is there (well, this is very useful for me).

Thanks

Piter PUNK

Re: [PATCH] Add compressed ID database support to (usb|pci)-db

[Piter PUNK]

On Wed, 30 Sep 2009, Lennart Poettering wrote:
> Ok, that makes sense to me. Sorry, Piter, it is unlikely that we will
> merge your patch.
>
> Even though your work will not be merged it is really appreciated!

Ok! Better luck for me next time -:)

Piter PUNK

Re: [PATCH] Add compressed ID database support to (usb|pci)-db

[Lennart Poettering]

On Tue, 29.09.09 16:41, Piter PUNK (piterpunk@slackware.com) wrote:
> On Wed, 30 Sep 2009, Lennart Poettering wrote:
> >Patch looks mstly good. However after discussing this a little with
> >Kay we'd prefer if we could do without the macro orgy and just depend
> >unconditionally on zlib. Given that this is in extras this should not
> >be a problem.
> 
> The macro orgy is from usb-utils code -;)
> 
> >Is there actually a distribution that uses a compressed pci.ids file?
> 
> Slackware uses it on installer but the udev use there is very
> restrict.

AFAIK currently the data generated by usb-db/pci-db is used only by
PulseAudio, NetworkManager and ModemManager. I am not sure how either
of these would matter during initialization.

Lennart

-- 
Lennart Poettering                        Red Hat, Inc.
lennart [at] poettering [dot] net
http://0pointer.net/lennart/           GnuPG 0x1A015CC4

Re: [PATCH] Add compressed ID database support to (usb|pci)-db

[Lennart Poettering]

On Wed, 30.09.09 02:03, Lennart Poettering (lennart@poettering.net) wrote:

> 
> On Tue, 29.09.09 16:41, Piter PUNK (piterpunk@slackware.com) wrote:
> > On Wed, 30 Sep 2009, Lennart Poettering wrote:
> > >Patch looks mstly good. However after discussing this a little with
> > >Kay we'd prefer if we could do without the macro orgy and just depend
> > >unconditionally on zlib. Given that this is in extras this should not
> > >be a problem.
> > 
> > The macro orgy is from usb-utils code -;)
> > 
> > >Is there actually a distribution that uses a compressed pci.ids file?
> > 
> > Slackware uses it on installer but the udev use there is very
> > restrict.
> 
> AFAIK currently the data generated by usb-db/pci-db is used only by
> PulseAudio, NetworkManager and ModemManager. I am not sure how either
> of these would matter during initialization.

s/initialization/installation/

Lennart

-- 
Lennart Poettering                        Red Hat, Inc.
lennart [at] poettering [dot] net
http://0pointer.net/lennart/           GnuPG 0x1A015CC4

Re: [PATCH] Add compressed ID database support to (usb|pci)-db

[Piter PUNK]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 566 bytes --]


On Wed, 30 Sep 2009, Lennart Poettering wrote:
>> Slackware uses it on installer but the udev use there is very
>> restrict.
>
> AFAIK currently the data generated by usb-db/pci-db is used only by
> PulseAudio, NetworkManager and ModemManager. I am not sure how either
> of these would matter during initialization.

Good. I know our udev use on installation is very limited, only to
create /dev and load network modules. Good to see that even if our
udev use increase, the (pci|usb).ids.gz support isn´t a problem
at all.

Thanks again!

Piter PUNK

Re: [PATCH] Add compressed ID database support to (usb|pci)-db

[Greg KH]

On Tue, Sep 29, 2009 at 04:41:21PM -0700, Piter PUNK wrote:
> 
> On Wed, 30 Sep 2009, Lennart Poettering wrote:
> > Patch looks mstly good. However after discussing this a little with
> > Kay we'd prefer if we could do without the macro orgy and just depend
> > unconditionally on zlib. Given that this is in extras this should not
> > be a problem.
> 
> The macro orgy is from usb-utils code -;)

And I'm seriously considering removing it from the usbutils package as
no distro ships compressed ids that I know of.

> > Is there actually a distribution that uses a compressed pci.ids file?
> 
> Slackware uses it on installer but the udev use there is very restrict.

Why would an installer need an .ids file?

thanks,

greg k-h

Re: [PATCH] Add compressed ID database support to (usb|pci)-db

[Piter PUNK]

On Tue, 29 Sep 2009, Greg KH wrote:
>>> Is there actually a distribution that uses a compressed pci.ids file?
>>
>> Slackware uses it on installer but the udev use there is very restrict.
>
> Why would an installer need an .ids file?

Because people wants to use lspci and lsusb to know the hardware they
are using and both commands search the names on .ids files. The lspci
helps me more than once to look some board and found tips to finnish
installation or fix some installer bug (usually some boot parameter
or kernel compilation option).

Piter PUNK

Re: [PATCH] Add compressed ID database support to (usb|pci)-db

[Frederic Crozat]

Le mardi 29 septembre 2009 à 18:56 -0700, Greg KH a écrit :
> On Tue, Sep 29, 2009 at 04:41:21PM -0700, Piter PUNK wrote:
> > 
> > On Wed, 30 Sep 2009, Lennart Poettering wrote:
> > > Patch looks mstly good. However after discussing this a little with
> > > Kay we'd prefer if we could do without the macro orgy and just depend
> > > unconditionally on zlib. Given that this is in extras this should not
> > > be a problem.
> > 
> > The macro orgy is from usb-utils code -;)
> 
> And I'm seriously considering removing it from the usbutils package as
> no distro ships compressed ids that I know of.

We used to do that (for pci.ids) but had to disable it when HAL started
to mmap the file and pciutils couldn't handle having file in both
compressed and uncompressed form.

-- 
Frederic Crozat <fcrozat@mandriva.com>
Mandriva

Re: [PATCH] Add compressed ID database support to (usb|pci)-db

[Greg KH]

On Tue, Sep 29, 2009 at 07:50:13PM -0700, Piter PUNK wrote:
> 
> 
> On Tue, 29 Sep 2009, Greg KH wrote:
> >>> Is there actually a distribution that uses a compressed pci.ids file?
> >>
> >> Slackware uses it on installer but the udev use there is very restrict.
> >
> > Why would an installer need an .ids file?
> 
> Because people wants to use lspci and lsusb to know the hardware they
> are using and both commands search the names on .ids files.

You should not need the id file for USB as the strings in the device are
almost always correct.  So you might wish to change your installer to
remove that dependancy.

thanks,

greg k-h