[Pcsclite-muscle] revamp libudev hotplug
Stefani Seibold
stefani
Mon Aug 4 04:08:11 PDT 2014
Hi,
this is a complete revamp of src/hotplug_libudev.c hotplug handling.
This patch fix a lot of race conditions and make the code a bit cleaner.
The following benefits will be achieved:
- udev events will be handled seperatly, a "add" event will only add a
device and a "remove" will now only remove the device. So the whole
bookkeeping is now done by the udev daemon, there is no more need to
track the state of the device. Also it fix a race where udev had yet not
finished the plug event and the device is not full accessible.
- fix the race gap between the coldplug scan and the activating of the
monitoring hotplug events. Now the monitoring will be first activated
and the the coldplug scan will be done. So there is a small change that
a device will be added during the coldplug scan. For this the
HPAddDevice() function needs to check for duplicate adds of a device.
But that is common design of libudev usage.
- No more mutex needed since there is no more concurrency, because the
coldplug scan will be no done inside the thread.
- Do not start the thread detached since there is no way to do a
synchronized stop of the thread. The HPStopHotPluggables() will now wait
until the thread is gone. So it is save to free memory of structures
since no one is access them.
- Remove the cooperative thread cancellation by AraKiriHotPlug. Despite
the non political correctness it will not work as expected, because the
select() system call will block infinitly. Using the pthread_cancel()
mechanism will do a better and safer job.
- Add signal safety to the select() system call. A EINTR should not lead
in a termination of the thread.
- Fix a typo: pluggble > plugable
- Make less error prone for unexpected libudev return values. Check all
return values of udev_...() functions against NULL.
I hope you like it.
- Stefani
Signed-off-by: Stefani Seibold <stefani at seibold.net>
hotplug_libudev.c | 327 +++++++++++++++++++++++++++---------------------------
1 file changed, 165 insertions(+), 162 deletions(-)
Index: src/hotplug_libudev.c
===================================================================
--- src/hotplug_libudev.c (revision 6947)
+++ src/hotplug_libudev.c (working copy)
@@ -3,6 +3,8 @@
*
* Copyright (C) 2011
* Ludovic Rousseau <ludovic.rousseau at free.fr>
+ * Copyright (C) 2014
+ * Stefani Seibold <stefani at seibold.net>
*
Redistribution and use in source and binary forms, with or without
modification, are permitted provided that the following conditions
@@ -35,7 +37,7 @@
/**
* @file
- * @brief This provides a search API for hot pluggble devices using libudev
+ * @brief This provides a search API for hot plugable devices using libudev
*/
#include "config.h"
@@ -56,6 +58,15 @@
#include "utils.h"
#include "strlcpycat.h"
+#ifndef TEMP_FAILURE_RETRY
+#define TEMP_FAILURE_RETRY(expression) \
+ (__extension__ \
+ ({ long int __result; \
+ do __result = (long int) (expression); \
+ while (__result == -1L && errno == EINTR); \
+ __result; }))
+#endif
+
#undef DEBUG_HOTPLUG
#define FALSE 0
@@ -64,11 +75,8 @@
extern char Add_Interface_In_Name;
extern char Add_Serial_In_Name;
-pthread_mutex_t usbNotifierMutex;
-
static pthread_t usbNotifyThread;
static int driverSize = -1;
-static char AraKiriHotPlug = FALSE;
/**
* keep track of drivers in a dynamically allocated array
@@ -89,21 +97,14 @@
* We start with a big array size to avoid reallocation. */
#define DRIVER_TRACKER_INITIAL_SIZE 200
-typedef enum {
- READER_ABSENT,
- READER_PRESENT,
- READER_FAILED
-} readerState_t;
-
/**
* keep track of PCSCLITE_MAX_READERS_CONTEXTS simultaneous readers
*/
static struct _readerTracker
{
- readerState_t status; /** reader state */
- char bInterfaceNumber; /** interface number on the device */
char *devpath; /**< device name seen by udev */
char *fullName; /**< full reader name (including serial number) */
+ char *sysname; /**< sysfs path */
} readerTracker[PCSCLITE_MAX_READERS_CONTEXTS];
@@ -321,10 +322,63 @@
}
-static void HPAddDevice(struct udev_device *dev, struct udev_device *parent,
- const char *devpath)
+static void HPRemoveDevice(struct udev_device *dev)
{
int i;
+ const char *devpath;
+ struct udev_device *parent;
+ const char *sysname;
+
+ /* The device pointed to by dev contains information about
+ the interface. In order to get information about the USB
+ device, get the parent device with the subsystem/devtype pair
+ of "usb"/"usb_device". This will be several levels up the
+ tree, but the function will find it.*/
+ parent = udev_device_get_parent_with_subsystem_devtype(dev, "usb",
+ "usb_device");
+ if (!parent)
+ return;
+
+ devpath = udev_device_get_devnode(parent);
+ if (!devpath)
+ {
+ /* the device disapeared? */
+ Log1(PCSC_LOG_ERROR, "udev_device_get_devnode() failed");
+ return;
+ }
+
+ sysname = udev_device_get_sysname(dev);
+ if (!sysname)
+ {
+ /* the device disapeared? */
+ Log1(PCSC_LOG_ERROR, "udev_device_get_sysname() failed");
+ return;
+ }
+
+ for (i=0; i<PCSCLITE_MAX_READERS_CONTEXTS; i++)
+ {
+ if (readerTracker[i].fullName && ! strcmp(sysname, readerTracker[i].sysname))
+ {
+ Log4(PCSC_LOG_INFO, "Removing USB device[%d]: %s at %s", i,
+ readerTracker[i].fullName, readerTracker[i].devpath);
+
+ RFRemoveReader(readerTracker[i].fullName,
+ PCSCLITE_HP_BASE_PORT + i);
+
+ free(readerTracker[i].devpath);
+ readerTracker[i].devpath = NULL;
+ free(readerTracker[i].fullName);
+ readerTracker[i].fullName = NULL;
+ free(readerTracker[i].sysname);
+ readerTracker[i].sysname = NULL;
+ }
+ }
+}
+
+
+static void HPAddDevice(struct udev_device *dev)
+{
+ int i;
char deviceName[MAX_DEVICENAME];
char fullname[MAX_READERNAME];
struct _driverTracker *driver, *classdriver;
@@ -332,7 +386,28 @@
const char *sInterfaceNumber;
LONG ret;
int bInterfaceNumber;
+ const char *devpath;
+ struct udev_device *parent;
+ const char *sysname;
+ /* The device pointed to by dev contains information about
+ the interface. In order to get information about the USB
+ device, get the parent device with the subsystem/devtype pair
+ of "usb"/"usb_device". This will be several levels up the
+ tree, but the function will find it.*/
+ parent = udev_device_get_parent_with_subsystem_devtype(dev, "usb",
+ "usb_device");
+ if (!parent)
+ return;
+
+ devpath = udev_device_get_devnode(parent);
+ if (!devpath)
+ {
+ /* the device disapeared? */
+ Log1(PCSC_LOG_ERROR, "udev_device_get_devnode() failed");
+ return;
+ }
+
driver = get_driver(parent, devpath, &classdriver);
if (NULL == driver)
{
@@ -344,6 +419,21 @@
return;
}
+ sysname = udev_device_get_sysname(dev);
+ if (!sysname)
+ {
+ /* the device disapeared? */
+ Log1(PCSC_LOG_ERROR, "udev_device_get_sysname() failed");
+ return;
+ }
+
+ /* check for duplicated add */
+ for (i=0; i<PCSCLITE_MAX_READERS_CONTEXTS; i++)
+ {
+ if (readerTracker[i].fullName && ! strcmp(sysname, readerTracker[i].sysname))
+ return;
+ }
+
Log2(PCSC_LOG_INFO, "Adding USB device: %s", driver->readerName);
sInterfaceNumber = udev_device_get_sysattr_value(dev, "bInterfaceNumber");
@@ -357,8 +447,6 @@
bInterfaceNumber, devpath);
deviceName[sizeof(deviceName) -1] = '\0';
- (void)pthread_mutex_lock(&usbNotifierMutex);
-
/* find a free entry */
for (i=0; i<PCSCLITE_MAX_READERS_CONTEXTS; i++)
{
@@ -370,7 +458,6 @@
{
Log2(PCSC_LOG_ERROR,
"Not enough reader entries. Already found %d readers", i);
- (void)pthread_mutex_unlock(&usbNotifierMutex);
return;
}
@@ -406,8 +493,7 @@
readerTracker[i].fullName = strdup(fullname);
readerTracker[i].devpath = strdup(devpath);
- readerTracker[i].status = READER_PRESENT;
- readerTracker[i].bInterfaceNumber = bInterfaceNumber;
+ readerTracker[i].sysname = strdup(sysname);
ret = RFAddReader(fullname, PCSCLITE_HP_BASE_PORT + i,
driver->libraryPath, deviceName);
@@ -425,34 +511,22 @@
{
Log2(PCSC_LOG_ERROR, "Failed adding USB device: %s",
driver->readerName);
-
- readerTracker[i].status = READER_FAILED;
-
(void)CheckForOpenCT();
}
}
else
{
- readerTracker[i].status = READER_FAILED;
-
(void)CheckForOpenCT();
}
}
-
- (void)pthread_mutex_unlock(&usbNotifierMutex);
} /* HPAddDevice */
-static void HPRescanUsbBus(struct udev *udev)
+static void HPScanUSB(struct udev *udev)
{
- int i, j;
struct udev_enumerate *enumerate;
struct udev_list_entry *devices, *dev_list_entry;
- /* all reader are marked absent */
- for (i=0; i < PCSCLITE_MAX_READERS_CONTEXTS; i++)
- readerTracker[i].status = READER_ABSENT;
-
/* Create a list of the devices in the 'usb' subsystem. */
enumerate = udev_enumerate_new(udev);
udev_enumerate_add_match_subsystem(enumerate, "usb");
@@ -462,12 +536,8 @@
/* For each item enumerated */
udev_list_entry_foreach(dev_list_entry, devices)
{
+ struct udev_device *dev;
const char *devpath;
- struct udev_device *dev, *parent;
- struct _driverTracker *driver, *classdriver;
- int newreader;
- int bInterfaceNumber;
- const char *interface;
/* Get the filename of the /sys entry for the device
and create a udev_device object (dev) representing it */
@@ -474,60 +544,11 @@
devpath = udev_list_entry_get_name(dev_list_entry);
dev = udev_device_new_from_syspath(udev, devpath);
- /* The device pointed to by dev contains information about
- the interface. In order to get information about the USB
- device, get the parent device with the subsystem/devtype pair
- of "usb"/"usb_device". This will be several levels up the
- tree, but the function will find it.*/
- parent = udev_device_get_parent_with_subsystem_devtype(dev, "usb",
- "usb_device");
- if (!parent)
- continue;
-
- devpath = udev_device_get_devnode(parent);
- if (!devpath)
- {
- /* the device disapeared? */
- Log1(PCSC_LOG_ERROR, "udev_device_get_devnode() failed");
- continue;
- }
-
- driver = get_driver(parent, devpath, &classdriver);
- if (NULL == driver)
- /* no driver known for this device */
- continue;
-
#ifdef DEBUG_HOTPLUG
Log2(PCSC_LOG_DEBUG, "Found matching USB device: %s", devpath);
#endif
+ HPAddDevice(dev);
- newreader = TRUE;
- bInterfaceNumber = 0;
- interface = udev_device_get_sysattr_value(dev, "bInterfaceNumber");
- if (interface)
- bInterfaceNumber = atoi(interface);
-
- /* Check if the reader is a new one */
- for (j=0; j<PCSCLITE_MAX_READERS_CONTEXTS; j++)
- {
- if (readerTracker[j].devpath
- && (strcmp(readerTracker[j].devpath, devpath) == 0)
- && (bInterfaceNumber == readerTracker[j].bInterfaceNumber))
- {
- /* The reader is already known */
- readerTracker[j].status = READER_PRESENT;
- newreader = FALSE;
-#ifdef DEBUG_HOTPLUG
- Log2(PCSC_LOG_DEBUG, "Refresh USB device: %s", devpath);
-#endif
- break;
- }
- }
-
- /* New reader found */
- if (newreader)
- HPAddDevice(dev, parent, devpath);
-
/* free device */
udev_device_unref(dev);
}
@@ -534,43 +555,43 @@
/* Free the enumerator object */
udev_enumerate_unref(enumerate);
+}
- pthread_mutex_lock(&usbNotifierMutex);
- /* check if all the previously found readers are still present */
- for (i=0; i<PCSCLITE_MAX_READERS_CONTEXTS; i++)
- {
- if ((READER_ABSENT == readerTracker[i].status)
- && (readerTracker[i].fullName != NULL))
- {
- Log4(PCSC_LOG_INFO, "Removing USB device[%d]: %s at %s", i,
- readerTracker[i].fullName, readerTracker[i].devpath);
- RFRemoveReader(readerTracker[i].fullName,
- PCSCLITE_HP_BASE_PORT + i);
+static void HPcleanup(void *ptr)
+{
+ int i;
- readerTracker[i].status = READER_ABSENT;
- free(readerTracker[i].devpath);
- readerTracker[i].devpath = NULL;
- free(readerTracker[i].fullName);
- readerTracker[i].fullName = NULL;
+ for (i=0; i<driverSize; i++)
+ {
+ /* free strings allocated by strdup() */
+ free(driverTracker[i].bundleName);
+ free(driverTracker[i].libraryPath);
+ free(driverTracker[i].readerName);
+ }
+ free(driverTracker);
- }
- }
- pthread_mutex_unlock(&usbNotifierMutex);
+ Log1(PCSC_LOG_INFO, "Hotplug stopped");
}
+
static void HPEstablishUSBNotifications(struct udev *udev)
{
struct udev_monitor *udev_monitor;
- int r, i;
+ int r;
int fd;
fd_set fds;
+ pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, NULL);
+ pthread_setcanceltype(PTHREAD_CANCEL_DEFERRED, NULL);
+
+ pthread_cleanup_push(HPcleanup, NULL);
+
udev_monitor = udev_monitor_new_from_netlink(udev, "udev");
if (NULL == udev_monitor)
{
Log1(PCSC_LOG_ERROR, "udev_monitor_new_from_netlink() error");
- return;
+ pthread_exit(NULL);
}
/* filter only the interfaces */
@@ -579,7 +600,7 @@
if (r)
{
Log2(PCSC_LOG_ERROR, "udev_monitor_filter_add_match_subsystem_devtype() error: %d\n", r);
- return;
+ pthread_exit(NULL);
}
r = udev_monitor_enable_receiving(udev_monitor);
@@ -586,7 +607,7 @@
if (r)
{
Log2(PCSC_LOG_ERROR, "udev_monitor_enable_receiving() error: %d\n", r);
- return;
+ pthread_exit(NULL);
}
/* udev monitor file descriptor */
@@ -594,13 +615,14 @@
if (fd < 0)
{
Log2(PCSC_LOG_ERROR, "udev_monitor_get_fd() error: %d", fd);
- return;
+ pthread_exit(NULL);
}
- while (!AraKiriHotPlug)
+ HPScanUSB(udev);
+
+ for (;;)
{
- struct udev_device *dev, *parent;
- const char *action, *devpath;
+ struct udev_device *dev;
#ifdef DEBUG_HOTPLUG
Log0(PCSC_LOG_INFO);
@@ -609,59 +631,46 @@
FD_ZERO(&fds);
FD_SET(fd, &fds);
+ pthread_setcancelstate(PTHREAD_CANCEL_ENABLE, NULL);
+
/* wait for a udev event */
- r = select(fd+1, &fds, NULL, NULL, NULL);
+ r = TEMP_FAILURE_RETRY(select(fd+1, &fds, NULL, NULL, NULL));
if (r < 0)
{
Log2(PCSC_LOG_ERROR, "select(): %s", strerror(errno));
- return;
+ pthread_exit(NULL);
}
+ pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, NULL);
+
dev = udev_monitor_receive_device(udev_monitor);
- if (!dev)
+ if (dev)
{
- Log1(PCSC_LOG_ERROR, "udev_monitor_receive_device() error\n");
- return;
- }
+ const char *action = udev_device_get_action(dev);
- action = udev_device_get_action(dev);
- if (0 == strcmp("remove", action))
- {
- Log1(PCSC_LOG_INFO, "Device removed");
- HPRescanUsbBus(udev);
- continue;
- }
+ if (action)
+ {
+ if (!strcmp("remove", action))
+ {
+ Log1(PCSC_LOG_INFO, "USB Device removed");
+ HPRemoveDevice(dev);
+ continue;
+ }
- if (strcmp("add", action))
- continue;
+ if (!strcmp("add", action)) {
+ Log1(PCSC_LOG_INFO, "USB Device add");
+ HPAddDevice(dev);
+ continue;
+ }
+ }
- parent = udev_device_get_parent_with_subsystem_devtype(dev, "usb",
- "usb_device");
- devpath = udev_device_get_devnode(parent);
- if (!devpath)
- {
- /* the device disapeared? */
- Log1(PCSC_LOG_ERROR, "udev_device_get_devnode() failed");
- continue;
+ /* free device */
+ udev_device_unref(dev);
}
-
- HPAddDevice(dev, parent, devpath);
-
- /* free device */
- udev_device_unref(dev);
-
}
- for (i=0; i<driverSize; i++)
- {
- /* free strings allocated by strdup() */
- free(driverTracker[i].bundleName);
- free(driverTracker[i].libraryPath);
- free(driverTracker[i].readerName);
- }
- free(driverTracker);
-
- Log1(PCSC_LOG_INFO, "Hotplug stopped");
+ pthread_cleanup_pop(1);
+ pthread_exit(NULL);
} /* HPEstablishUSBNotifications */
@@ -674,8 +683,6 @@
for (i=0; i<PCSCLITE_MAX_READERS_CONTEXTS; i++)
{
- readerTracker[i].status = READER_ABSENT;
- readerTracker[i].bInterfaceNumber = 0;
readerTracker[i].devpath = NULL;
readerTracker[i].fullName = NULL;
}
@@ -689,8 +696,8 @@
*/
LONG HPStopHotPluggables(void)
{
- AraKiriHotPlug = TRUE;
-
+ pthread_cancel(usbNotifyThread);
+ pthread_join(usbNotifyThread, NULL);
return 0;
} /* HPStopHotPluggables */
@@ -702,8 +709,6 @@
{
struct udev *udev;
- (void)pthread_mutex_init(&usbNotifierMutex, NULL);
-
if (driverSize <= 0)
{
Log1(PCSC_LOG_INFO, "No bundle files in pcsc drivers directory: "
@@ -720,9 +725,7 @@
return 0;
}
- HPRescanUsbBus(udev);
-
- (void)ThreadCreate(&usbNotifyThread, THREAD_ATTR_DETACHED,
+ (void)ThreadCreate(&usbNotifyThread, 0,
(PCSCLITE_THREAD_FUNCTION( )) HPEstablishUSBNotifications, udev);
return 0;
More information about the pcsclite-muscle
mailing list