libpci for PCI IDs parsing (again)
Daniel Drake
dsd at gentoo.org
Sun Jul 8 19:11:51 PDT 2007
David,
What is your opinion on the latest version of this patch (inlined below)?
Here is my analysis on the performance and memory effects of this patch:
http://lists.freedesktop.org/archives/hal/2007-June/008852.html
plus it appears that zlib is nothing like as expensive as initially suspected,
maybe people were confusing zlib with bzip2?
(even so, the plaintext vs .gz choice is left to the user or distro)
http://lists.freedesktop.org/archives/hal/2007-June/008861.html
Assuming this kind of approach is acceptable, are we waiting for pciutils
to provide a shared library or can this be merged at an earlier time?
Patch below is from Mike Frysinger <vapier at gentoo.org>
diff --git a/configure.in b/configure.in
index 0b6734c..647c0c6 100644
--- a/configure.in
+++ b/configure.in
@@ -46,20 +46,10 @@ AC_ARG_WITH([pid-file],
AS_HELP_STRING([--with-pid-file=<file>],
[PID file for HAL daemon]))
-AC_ARG_WITH([hwdata],
- AS_HELP_STRING([--with-hwdata=<dir>],
- [Where PCI and USB IDs are found]))
-AC_ARG_WITH([pci-ids],
- AS_HELP_STRING([--with-pci-ids=<dir>],
- [Where PCI IDs are found (overrides --with-hwdata)]))
AC_ARG_WITH([usb-ids],
AS_HELP_STRING([--with-usb-ids=<dir>],
[Where USB IDs are found (overrides --with-hwdata)]))
-AC_ARG_ENABLE([pci-ids],
- AS_HELP_STRING([--disable-pci-ids],
- [Do not build with PCI IDs support]),
- [enable_pci_ids=$enableval], [enable_pci_ids=yes])
AC_ARG_ENABLE([usb-ids],
AS_HELP_STRING([--disable-usb-ids],
[Do not build with USB IDs support]),
@@ -73,34 +63,10 @@ AC_ARG_WITH([socket-dir],
AS_HELP_STRING([--with-socket-dir=<dir>],
[Location of the HAL D-BUS listening sockets (auto)]))
-if ! test -z "$with_hwdata" ; then
- PCI_IDS_DIR="$with_hwdata"
- USB_IDS_DIR="$with_hwdata"
-fi
-if ! test -z "$with_pci_ids" ; then
- PCI_IDS_DIR="$with_pci_ids"
-fi
if ! test -z "$with_usb_ids" ; then
USB_IDS_DIR="$with_usb_ids"
fi
-if test "x$enable_pci_ids" = "xno" ; then
- USE_PCI_IDS=no
-else
- if test -z "$PCI_IDS_DIR"; then
- for dir in /usr/share/hwdata /usr/share/misc /usr/share /var/lib/misc; do
- AC_CHECK_FILE([$dir/pci.ids], [PCI_IDS_DIR=$dir])
- done
- if test -z "$PCI_IDS_DIR"; then
- AC_MSG_ERROR([cannot find pci.ids. Use --with-pci-ids to specify location])
- else
- AC_MSG_WARN([autodetected pci.ids in $PCI_IDS_DIR])
- fi
- fi
- USE_PCI_IDS=yes
- AC_DEFINE([USE_PCI_IDS], [1], [Whether pci.ids is to be used])
-fi
-
if test "x$enable_usb_ids" = "xno" ; then
USE_USB_IDS=no
else
@@ -125,9 +91,7 @@ else
AC_DEFINE([USE_PNP_IDS], [1], [Whether builtin PNP IDs are to be used])
fi
-AC_SUBST(PCI_IDS_DIR)
AC_SUBST(USB_IDS_DIR)
-AC_SUBST(USE_PCI_IDS)
AC_SUBST(USE_USB_IDS)
AC_SUBST(USE_PNP_IDS)
@@ -422,11 +386,13 @@ AC_ARG_WITH([libpci],
[], [with_libpci=yes])
USE_LIBPCI="no"
+PCI_LIBS=""
if test "x$with_libpci" != xno ; then
dnl check for libpci
AC_CHECK_HEADERS([pci/pci.h],
- [AC_CHECK_LIB([pci], [pci_init], [USE_LIBPCI="yes"], [], [-lz])])
-fi
+ [AC_CHECK_LIB([pci], [pci_init], [USE_LIBPCI="yes" PCI_LIBS="-lpci -lz"], [], [-lz])])
+fi
+AC_SUBST([PCI_LIBS])
AM_CONDITIONAL([HAVE_LIBPCI], [test "x$USE_LIBPCI" = "xyes"])
AC_ARG_WITH([backend],
@@ -979,7 +945,6 @@ echo "
localstatedir: ${localstatedir}
docdir: ${docdir}
dbus-1 system.d dir: ${DBUS_SYS_DIR}
- pci.ids dir: ${PCI_IDS_DIR}
usb.ids dir: ${USB_IDS_DIR}
compiler: ${CC}
diff --git a/hald/Makefile.am b/hald/Makefile.am
index ea396f6..fbf2d5d 100644
--- a/hald/Makefile.am
+++ b/hald/Makefile.am
@@ -12,7 +12,6 @@ AM_CPPFLAGS = \
-DPACKAGE_SCRIPT_DIR=\""$(libdir)/hal/scripts"\" \
-DHALD_SOCKET_DIR=\""$(HALD_SOCKET_DIR)"\" \
-DHALD_PID_FILE=\""$(HALD_PID_FILE)"\" \
- -DPCI_IDS_DIR=\""$(PCI_IDS_DIR)"\" \
-DUSB_IDS_DIR=\""$(USB_IDS_DIR)"\" \
-I$(top_srcdir) \
@GLIB_CFLAGS@ @DBUS_CFLAGS@ @POLKIT_CFLAGS@
@@ -73,7 +72,7 @@ if HAVE_CONKIT
hald_SOURCES += ck-tracker.h ck-tracker.c
endif
-hald_LDADD = @GLIB_LIBS@ @DBUS_LIBS@ @POLKIT_LIBS@ -lm @HALD_OS_LIBS@ $(top_builddir)/hald/$(HALD_BACKEND)/libhald_$(HALD_BACKEND).la
+hald_LDADD = @PCI_LIBS@ @GLIB_LIBS@ @DBUS_LIBS@ @POLKIT_LIBS@ -lm @HALD_OS_LIBS@ $(top_builddir)/hald/$(HALD_BACKEND)/libhald_$(HALD_BACKEND).la
#### Init scripts fun
SCRIPT_IN_FILES=haldaemon.in
diff --git a/hald/ids.c b/hald/ids.c
index 9949eab..b8dc344 100644
--- a/hald/ids.c
+++ b/hald/ids.c
@@ -44,72 +44,17 @@
#include "ids.h"
-#ifdef USE_PCI_IDS
-/** Pointer to where the pci.ids file is loaded */
-static char *pci_ids = NULL;
-
-/** Length of data store at at pci_ids */
-static size_t pci_ids_len;
+#ifdef HAVE_PCI
-/** Iterator position into pci_ids */
-static size_t pci_ids_iter_pos;
+#include <pci/pci.h>
-/** Initialize the pci.ids line iterator to the beginning of the file */
-static void
-pci_ids_line_iter_init ()
-{
- pci_ids_iter_pos = 0;
-}
+/** Pointer to where the pci.ids file is loaded */
+static struct pci_access *pacc = NULL;
/** Maximum length of lines in pci.ids */
#define PCI_IDS_MAX_LINE_LEN 512
-/**
- * pci_ids_line_iter_get_line:
- * @line_len: Pointer to where number of bytes in line will
- * be stored
- * Returns: Pointer to the line; only valid until the
- * next invocation of this function
- *
- * Get the next line from pci.ids
- */
-static char *
-pci_ids_line_iter_get_line (unsigned int *line_len)
-{
- unsigned int i;
- static char line[PCI_IDS_MAX_LINE_LEN];
-
- for (i = 0;
- pci_ids_iter_pos < pci_ids_len &&
- i < PCI_IDS_MAX_LINE_LEN - 1 &&
- pci_ids[pci_ids_iter_pos] != '\n'; i++, pci_ids_iter_pos++) {
- line[i] = pci_ids[pci_ids_iter_pos];
- }
-
- line[i] = '\0';
- if (line_len != NULL)
- *line_len = i;
-
- pci_ids_iter_pos++;
-
- return line;
-}
-
-/**
- * pci_ids_line_iter_has_more:
- *
- * Returns: #TRUE iff there are more lines to process
- *
- * See if there are more lines to process in pci.ids
- */
-static dbus_bool_t
-pci_ids_line_iter_has_more ()
-{
- return pci_ids_iter_pos < pci_ids_len;
-}
-
-
-/**
+/**
* ids_find_pci:
* @vendor_id: PCI vendor id or 0 if unknown
* @product_id: PCI product id or 0 if unknown
@@ -131,199 +76,64 @@ ids_find_pci (int vendor_id, int product_id,
char **vendor_name, char **product_name,
char **subsys_vendor_name, char **subsys_product_name)
{
- char *line;
- unsigned int i;
- unsigned int line_len;
- unsigned int num_tabs;
- char rep_vi[8];
- char rep_pi[8];
- char rep_svi[8];
- char rep_spi[8];
- dbus_bool_t vendor_matched = FALSE;
- dbus_bool_t product_matched = FALSE;
static char store_vn[PCI_IDS_MAX_LINE_LEN];
static char store_pn[PCI_IDS_MAX_LINE_LEN];
static char store_svn[PCI_IDS_MAX_LINE_LEN];
static char store_spn[PCI_IDS_MAX_LINE_LEN];
- snprintf (rep_vi, 8, "%04x", vendor_id);
- snprintf (rep_pi, 8, "%04x", product_id);
- snprintf (rep_svi, 8, "%04x", subsys_vendor_id);
- snprintf (rep_spi, 8, "%04x", subsys_product_id);
-
*vendor_name = NULL;
*product_name = NULL;
*subsys_vendor_name = NULL;
*subsys_product_name = NULL;
- for (pci_ids_line_iter_init (); pci_ids_line_iter_has_more ();) {
- line = pci_ids_line_iter_get_line (&line_len);
-
- /* skip lines with no content */
- if (line_len < 4)
- continue;
-
- /* skip comments */
- if (line[0] == '#')
- continue;
-
- /* count number of tabs */
- num_tabs = 0;
- for (i = 0; i < line_len; i++) {
- if (line[i] != '\t')
- break;
- num_tabs++;
- }
-
- switch (num_tabs) {
- case 0:
- /* vendor names */
- vendor_matched = FALSE;
-
- /* first check subsys_vendor_id, if haven't done
- * already */
- if (*subsys_vendor_name == NULL
- && subsys_vendor_id != 0) {
- if ((*((dbus_uint32_t *) line)) ==
- (*((dbus_uint32_t *) rep_svi))) {
- /* found it */
- for (i = 4; i < line_len; i++) {
- if (!isspace (line[i]))
- break;
- }
- strncpy (store_svn, line + i,
- PCI_IDS_MAX_LINE_LEN);
- *subsys_vendor_name = store_svn;
- }
- }
-
- /* check vendor_id */
- if (vendor_id != 0) {
- if (memcmp (line, rep_vi, 4) == 0) {
- /* found it */
- vendor_matched = TRUE;
-
- for (i = 4; i < line_len; i++) {
- if (!isspace (line[i]))
- break;
- }
- strncpy (store_vn, line + i,
- PCI_IDS_MAX_LINE_LEN);
- *vendor_name = store_vn;
- }
- }
-
- break;
-
- case 1:
- product_matched = FALSE;
-
- /* product names */
- if (!vendor_matched)
- continue;
-
- /* check product_id */
- if (product_id != 0) {
- if (memcmp (line + 1, rep_pi, 4) == 0) {
- /* found it */
-
- product_matched = TRUE;
-
- for (i = 5; i < line_len; i++) {
- if (!isspace (line[i]))
- break;
- }
- strncpy (store_pn, line + i,
- PCI_IDS_MAX_LINE_LEN);
- *product_name = store_pn;
- }
- }
- break;
-
- case 2:
- /* subsystem_vendor subsystem_product */
- if (!vendor_matched || !product_matched)
- continue;
-
- /* check product_id */
- if (subsys_vendor_id != 0
- && subsys_product_id != 0) {
- if (memcmp (line + 2, rep_svi, 4) == 0
- && memcmp (line + 7, rep_spi,
- 4) == 0) {
- /* found it */
- for (i = 11; i < line_len; i++) {
- if (!isspace (line[i]))
- break;
- }
- strncpy (store_spn, line + i,
- PCI_IDS_MAX_LINE_LEN);
- *subsys_product_name = store_spn;
- }
- }
-
- break;
-
- default:
- break;
- }
-
- }
+ if (vendor_id == 0)
+ return;
+ *vendor_name = pci_lookup_name (pacc, store_vn, sizeof(store_vn),
+ PCI_LOOKUP_VENDOR, vendor_id, product_id);
+ if (*vendor_name == NULL)
+ return;
+
+ if (product_id == 0)
+ return;
+ *product_name = pci_lookup_name (pacc, store_pn, sizeof(store_pn),
+ PCI_LOOKUP_DEVICE, vendor_id, product_id);
+ if (*product_name == NULL)
+ return;
+
+ if (subsys_vendor_id == 0)
+ return;
+ *subsys_vendor_name = pci_lookup_name (pacc, store_svn, sizeof(store_svn),
+ PCI_LOOKUP_SUBSYSTEM | PCI_LOOKUP_VENDOR, subsys_vendor_id);
+ if (*subsys_vendor_name == NULL)
+ return;
+
+ if (subsys_product_id == 0)
+ return;
+ *subsys_product_name = pci_lookup_name (pacc, store_spn, sizeof(store_spn),
+ PCI_LOOKUP_SUBSYSTEM | PCI_LOOKUP_DEVICE, subsys_vendor_id, subsys_product_id);
+ if (*subsys_product_name == NULL)
+ return;
}
-
-/**
- * pci_ids_load:
- * @path: Path of the pci.ids file, e.g. /usr/share/hwdata/pci.ids
- *
- * Returns: #TRUE if the file was succesfully loaded
- *
- * Load the PCI database used for mapping vendor, product, subsys_vendor
- * and subsys_product numbers into names.
- */
-static dbus_bool_t
-pci_ids_load (const char *path)
+static void
+pci_ids_error (char *msg, ...)
{
- int fd;
- struct stat statbuf;
- gboolean ret;
-
- ret = FALSE;
-
- if (stat (path, &statbuf) != 0) {
- HAL_WARNING (("Couldn't stat pci.ids file '%s', errno=%d: %s", path, errno, strerror (errno)));
- goto out;
- }
- pci_ids_len = statbuf.st_size;
-
- fd = open (path, O_RDONLY);
- if (fd < 0) {
- HAL_WARNING (("Couldn't open pci.ids file '%s', errno=%d: %s", path, errno, strerror (errno)));
- goto out;
- }
-
- pci_ids = mmap (NULL, pci_ids_len, PROT_READ, MAP_SHARED, fd, 0);
- if (pci_ids == MAP_FAILED) {
- HAL_WARNING (("Couldn't mmap pci.ids file '%s', errno=%d: %s", path, errno, strerror (errno)));
- close (fd);
- goto out;
- }
-
- ret = TRUE;
-
- close (fd);
-out:
- return ret;
+ va_list args;
+ va_start (args, msg);
+ logger_setup (HAL_LOGPRI_ERROR, __FILE__, __LINE__, __FUNCTION__);
+ logger_emit_var (msg, args);
+ va_end (args);
}
void
pci_ids_init (void)
{
- /* Load /usr/share/hwdata/pci.ids */
- pci_ids_load (PCI_IDS_DIR "/pci.ids");
+ pacc = pci_alloc();
+ pacc->error = pci_ids_error;
+ pci_init(pacc);
}
-#endif /*USE_PCI_IDS*/
+#endif /*HAVE_PCI*/
/*==========================================================================*/
diff --git a/hald/ids.h b/hald/ids.h
index 7773211..e0a8679 100644
--- a/hald/ids.h
+++ b/hald/ids.h
@@ -28,7 +28,7 @@
#include <glib.h>
-#ifdef USE_PCI_IDS
+#ifdef HAVE_PCI
void pci_ids_init (void);
@@ -38,7 +38,7 @@ ids_find_pci (int vendor_id, int product_id,
char **vendor_name, char **product_name,
char **subsys_vendor_name, char **subsys_product_name);
-#else /*USE_PCI_IDS*/
+#else /*HAVE_PCI*/
static inline void pci_ids_init (void) {return;};
static inline void
@@ -46,7 +46,7 @@ ids_find_pci (int vendor_id, int product_id,
int subsys_vendor_id, int subsys_product_id,
char **vendor_name, char **product_name,
char **subsys_vendor_name, char **subsys_product_name) {return;}
-#endif /*USE_PCI_IDS*/
+#endif /*HAVE_PCI*/
#ifdef USE_PNP_IDS
diff --git a/hald/logger.c b/hald/logger.c
index a2d04b5..c23ea71 100644
--- a/hald/logger.c
+++ b/hald/logger.c
@@ -138,16 +138,15 @@ logger_setup (int _priority, const char *_file, int _line, const char *_function
}
/**
- * logger_emit:
+ * logger_emit_var:
* @format: Message format string, printf style
- * @...: Parameters for message, printf style
+ * @args: Parameters for message, vprintf style
*
* Emit logging entry
*/
void
-logger_emit (const char *format, ...)
+logger_emit_var (const char *format, va_list args)
{
- va_list args;
char buf[512];
char *pri;
char tbuf[256];
@@ -160,7 +159,6 @@ logger_emit (const char *format, ...)
if (!is_enabled)
return;
- va_start (args, format);
vsnprintf (buf, sizeof (buf), format, args);
switch (priority) {
@@ -213,7 +211,21 @@ logger_emit (const char *format, ...)
break;
}
}
+}
+/**
+ * loger_emit:
+ * @format: Message format string, printf style
+ * @...: Parameters for message, printf style
+ *
+ * Emit logging entry
+ */
+void
+logger_emit (const char *format, ...)
+{
+ va_list args;
+ va_start (args, format);
+ logger_emit_var (format, args);
va_end (args);
}
diff --git a/hald/logger.h b/hald/logger.h
index f32216c..dd55eb5 100644
--- a/hald/logger.h
+++ b/hald/logger.h
@@ -30,6 +30,7 @@ extern "C" {
#ifndef LOGGER_H
#define LOGGER_H
+#include <stdarg.h>
#include <stdio.h>
#include <stdlib.h>
@@ -45,6 +46,7 @@ enum {
void logger_setup (int priority, const char *file, int line, const char *function);
void logger_emit (const char *format, ...);
+void logger_emit_var (const char *format, va_list args);
/* void logger_forward_debug (const char *format, va_list args); */
void logger_forward_debug (const char *format, ...);
@@ -72,7 +74,7 @@ void setup_logger (void);
/** Warning level logging macro */
#define HAL_WARNING(expr) do {logger_setup(HAL_LOGPRI_WARNING, __FILE__, __LINE__, __FUNCTION__); logger_emit expr; } while(0)
-/** Error leve logging macro */
+/** Error level logging macro */
#define HAL_ERROR(expr) do {logger_setup(HAL_LOGPRI_ERROR, __FILE__, __LINE__, __FUNCTION__); logger_emit expr; } while(0)
/** Macro for terminating the program on an unrecoverable error */
More information about the hal
mailing list