[libhid-discuss] Internal definition of struct usb_dev_handle
breaks portability
Albert Lee
trisk at jhu.edu
Sat Mar 31 01:22:02 UTC 2007
On Thu, 29 Mar 2007, Charles Lepple wrote:
> On Mar 29, 2007, at 8:33 PM, Albert Lee wrote:
>
>> I noticed problems building libhid on Solaris because libhid makes
>> assumptions about struct usb_dev_handle, which it keeps internal copies of.
>> This is fairly bad. Is there a reason not to use usb_device() to get the
>> device from the handle instead of poking at the struct contents?
>
> I don't know of any reasons offhand, but then again, I think Martin or Arnaud
> wrote that code (not to pass the buck - it's just that none of us were really
> doing all that much with Solaris).
>
> Can you post some of the errors?
>
Actually, it compiled and linked (after some build system changes), but
segfaulted at runtime since the struct contents were different from the
private copy's. libusb on Solaris doesn't actually expose the definition
of usb_dev_handle, but the interface accepts pointers to it. In the
attached patch I have changed the code to just use struct usb_device.
-Albert
-------------- next part --------------
Index: test/lshid.c
===================================================================
--- test/lshid.c (revision 318)
+++ test/lshid.c (working copy)
@@ -27,29 +27,22 @@
char *hid_id[32]; /* FIXME: 32 devices MAX */
-/* NOTE: included from libusb/usbi.h. UGLY, i know, but so is libusb! */
-struct usb_dev_handle {
- int fd;
- struct usb_bus *bus;
- struct usb_device *device;
- int config;
- int interface;
- int altsetting;
- void *impl_info;
-};
+struct usb_dev_handle;
bool device_iterator (struct usb_dev_handle const* usbdev, void* custom, unsigned int len)
{
bool ret = false;
int i;
char current_dev_path[10];
+ const struct usb_device *device = usb_device((struct usb_dev_handle *)usbdev);
/* only here to prevent the unused warning */
/* TODO remove */
len = *((unsigned long*)custom);
/* Obtain the device's full path */
- sprintf(current_dev_path, "%s/%s", usbdev->bus->dirname, usbdev->device->filename);
+ //sprintf(current_dev_path, "%s/%s", usbdev->bus->dirname, usbdev->device->filename);
+ sprintf(current_dev_path, "%s/%s", device->bus->dirname, device->filename);
/* Check if we already saw this dev */
for ( i = 0 ; ( hid_id[i] != NULL ) ; i++ )
@@ -61,8 +54,8 @@
/* Append device to the list if needed */
if (hid_id[i] == NULL)
{
- hid_id[i] = (char *) malloc (strlen(usbdev->device->filename) + strlen(usbdev->bus->dirname) );
- sprintf(hid_id[i], "%s/%s", usbdev->bus->dirname, usbdev->device->filename);
+ hid_id[i] = (char *) malloc (strlen(device->filename) + strlen(device->bus->dirname) );
+ sprintf(hid_id[i], "%s/%s", device->bus->dirname, device->filename);
}
else /* device already seen */
{
@@ -70,9 +63,9 @@
}
/* Filter non HID device */
- if ( (usbdev->device->descriptor.bDeviceClass == 0) /* Class defined at interface level */
- && usbdev->device->config
- && usbdev->device->config->interface->altsetting->bInterfaceClass == USB_CLASS_HID)
+ if ( (device->descriptor.bDeviceClass == 0) /* Class defined at interface level */
+ && device->config
+ && device->config->interface->altsetting->bInterfaceClass == USB_CLASS_HID)
ret = true;
else
ret = false;
Index: m4/md_check_os.m4
===================================================================
--- m4/md_check_os.m4 (revision 318)
+++ m4/md_check_os.m4 (working copy)
@@ -15,6 +15,12 @@
MD_OS=bsd
AC_MSG_RESULT(*BSD)
;;
+ *-solaris*)
+ AC_DEFINE(OS_SOLARIS, [], [define to 1 if building for Solaris])
+ AC_SUBST(OS_SOLARIS)
+ MD_OS=solaris
+ AC_MSG_RESULT(Solaris)
+ ;;
*-darwin*)
AC_DEFINE(OS_DARWIN, [], [define to 1 if building for OS X (Darwin)])
AC_SUBST(OS_DARWIN)
Index: src/debug.c
===================================================================
--- src/debug.c (revision 318)
+++ src/debug.c (working copy)
@@ -21,16 +21,7 @@
usb_set_debug(level);
}
-/* NOTE: included from libusb/usbi.h. UGLY, i know, but so is libusb! */
-struct usb_dev_handle {
- int fd;
- struct usb_bus *bus;
- struct usb_device *device;
- int config;
- int interface;
- int altsetting;
- void *impl_info;
-};
+struct usb_dev_handle;
void trace_usb_bus(FILE* out, struct usb_bus const* usbbus)
{
@@ -87,14 +78,8 @@
void trace_usb_dev_handle(FILE* out, usb_dev_handle const* usbdev_h)
{
- fprintf(out, "usb_dev_handle instance at: %10p\n", usbdev_h);
- fprintf(out, " fd: %d\n", usbdev_h->fd);
- fprintf(out, " bus: %10p\n", usbdev_h->bus);
- fprintf(out, " device: %10p\n", usbdev_h->device);
- fprintf(out, " config: %d\n", usbdev_h->config);
- fprintf(out, " interface: %d\n", usbdev_h->interface);
- fprintf(out, " altsetting: %d\n", usbdev_h->altsetting);
- fprintf(out, " impl_info: %10p\n", usbdev_h->impl_info);
+ struct usb_device *device = usb_device((usb_dev_handle *)usbdev_h);
+ trace_usb_device(out, device);
}
/* COPYRIGHT --
Index: src/Makefile.am
===================================================================
--- src/Makefile.am (revision 318)
+++ src/Makefile.am (working copy)
@@ -11,6 +11,9 @@
if OS_BSD
OS_SUPPORT_SOURCE = bsd.c
else
+if OS_SOLARIS
+OS_SUPPORT_SOURCE = bsd.c
+else
if OS_DARWIN
OS_SUPPORT_SOURCE = darwin.c
AM_CFLAGS += -no-cpp-precomp
@@ -20,6 +23,7 @@
endif
endif
endif
+endif
lib_LTLIBRARIES = libhid.la
libhid_la_DEPENDENCIES = ../hidparser/libhidparser.la
Index: configure.ac
===================================================================
--- configure.ac (revision 318)
+++ configure.ac (working copy)
@@ -15,7 +15,7 @@
m4_define([LIBHID_VERSION], [0.2.16])
m4_define([IFACE_AGE], [0])
m4_define([BIN_AGE], [0])
-m4_define([MD_INIT_NAME], m4_esyscmd([set -- `head -1 README` && echo -n $1]))
+m4_define([MD_INIT_NAME], m4_esyscmd([set -- `head -1 README` && echo $1 | tr -d "\n"]))
m4_define([MD_INIT_VERSION], LIBHID_VERSION.IFACE_AGE.BIN_AGE)
m4_define([MD_INIT_ADDRESS], m4_esyscmd([grep -i bugreports\ to README | sed -e 's,.*<\([^>]*\).*,\1,' | tr -d "\n"]))
@@ -79,6 +79,7 @@
MD_CONF_COMPILER
AM_CONDITIONAL(OS_LINUX, test "$MD_OS" = "linux")
AM_CONDITIONAL(OS_BSD, test "$MD_OS" = "bsd")
+AM_CONDITIONAL(OS_SOLARIS, test "$MD_OS" = "solaris")
AM_CONDITIONAL(OS_DARWIN, test "$MD_OS" = "darwin")
MD_CHECK_SWIG([1.3])
More information about the libhid-discuss
mailing list