Re: [Libusbx-devel] umask affects libusb_attach_kernel_driver()

[Hans de Goede]

Hi

On 06/26/2013 09:17 PM, Chris Dickens wrote:
> Yes, devtmpfs. Linux Kernel is 2.6.32-358 (RHEL6).

Ok likely a kernel issue then. You should probably retry with
a more recent kernel (ie try on a Fedora 18 install), and then
file a bug either against the upstream kernel, or against RHEL-6.

Regards,

Hans

--
This SF.net email is sponsored by Windows:

Build for Windows Store.

http://p.sf.net/sfu/windows-dev2dev
___
libusbx-devel mailing list
libusbx-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libusbx-devel

Re: [Libusbx-devel] Improving linux hotplug disconnect

[Hans de Goede]

Hi,

On 06/26/2013 06:57 PM, Chris Dickens wrote:
> Hi,
>
> On Wed, Jun 26, 2013 at 8:34 AM, Hans de Goede  > wrote:
>
> Hi,
>
> Hmm, the libusb_poll call in get_device_list() should make this quite 
> hard to trigger, this will only
> happen if udev has not yet read the kernel event, and send an event on 
> its own socket. Which likely
> means that your setup is somehow cpu starved.
>
>
> Are you talking about the hotplug_poll() call? My system has two Intel Xeon 
> X5560 CPUs, for a total of 16 cores. I'd be very surprised if this were a 
> resource issue. The application at the time this occurs is running between 6 
> and 8 threads, and there are plenty of blocking calls in these threads to 
> allow others to be scheduled. To verify this, I added a short timeout (10ms) 
> to the linux_udev_event_thread_main() function's poll() call and added a 
> usbi_dbg() call in the loop, and I see that the thread is getting scheduled 
> regularly before the udev monitor event is received. Given this, I think
> it may be safe to rule out CPU starvation. For whatever reason, the udev 
> event is just not being delivered on the socket.
>
> Although I'm not really a fan of having 2 device removed codepaths, I 
> think this is probably a
> simple and useful patch. Can you post the patch for review please?
>
>
> diff --git a/libusb/os/linux_netlink.c b/libusb/os/linux_netlink.c
> index 3a68f69..6581ebb 100644
> --- a/libusb/os/linux_netlink.c
> +++ b/libusb/os/linux_netlink.c
> @@ -214,7 +214,7 @@ static int linux_netlink_read_message(void)
> /* signal device is available (or not) to all contexts */
> if (detached)
> -linux_hotplug_disconnected(busnum, devaddr, sys_name);
> +linux_device_disconnected(busnum, devaddr, sys_name);
> else
> linux_hotplug_enumerate(busnum, devaddr, sys_name);
> diff --git a/libusb/os/linux_udev.c b/libusb/os/linux_udev.c
> index 5a2aadf..08b2d70 100644
> --- a/libusb/os/linux_udev.c
> +++ b/libusb/os/linux_udev.c
> @@ -207,7 +207,7 @@ static void udev_hotplug_event(struct udev_device* 
> udev_dev)
> if (strncmp(udev_action, "add", 3) == 0) {
> linux_hotplug_enumerate(busnum, devaddr, sys_name);
> } else if (detached) {
> -linux_hotplug_disconnected(busnum, devaddr, sys_name);
> +linux_device_disconnected(busnum, devaddr, sys_name);
> } else {
> usbi_err(NULL, "ignoring udev action %s", udev_action);
> }
> diff --git a/libusb/os/linux_usbfs.c b/libusb/os/linux_usbfs.c
> index 09288af..50f2d22 100644
> --- a/libusb/os/linux_usbfs.c
> +++ b/libusb/os/linux_usbfs.c
> @@ -1072,7 +1072,7 @@ void linux_hotplug_enumerate(uint8_t busnum, uint8_t 
> devaddr, const char *sys_na
> usbi_mutex_static_unlock(&active_contexts_lock);
>   }
> -void linux_hotplug_disconnected(uint8_t busnum, uint8_t devaddr, const char 
> *sys_name)
> +void linux_device_disconnected(uint8_t busnum, uint8_t devaddr, const char 
> *sys_name)
>   {
> struct libusb_context *ctx;
> struct libusb_device *dev;
> @@ -1247,8 +1247,18 @@ static int op_open(struct libusb_device_handle *handle)
> int r;
> hpriv->fd = _get_usbfs_fd(handle->dev, O_RDWR, 0);
> -if (hpriv->fd < 0)
> +if (hpriv->fd < 0) {
> +if (hpriv->fd == LIBUSB_ERROR_NO_DEVICE) {

> +/* device will still be marked as attached if hotplug monitor thread
> +* hasn't processed remove event yet */
> +if (handle->dev->attached) {

This check is racy, you need to take the hotplug lock before making this
check, also linux_device_disconnected expects the hotplug lock to be
taken when it is called, so simply but a check around the entire block.

> +usbi_dbg("open failed with no device, but device still attached");
> +linux_device_disconnected(handle->dev->bus_number,
> +handle->dev->device_address, NULL);
> +}
> +}
> return hpriv->fd;
> +}
> r = ioctl(hpriv->fd, IOCTL_USBFS_GET_CAPABILITIES, &hpriv->caps);
> if (r < 0) {
> @@ -2482,6 +2492,11 @@ static int op_handle_events(struct libusb_context *ctx,
> if (pollfd->revents & POLLERR) {
> usbi_remove_pollfd(HANDLE_CTX(handle), hpriv->fd);
> usbi_handle_disconnect(handle);
> +/* device will still be marked as attached if hotplug monitor thread
> +* hasn't processed remove event yet */
> +if (handle->dev->attached)
> +linux_device_disconnected(handle->dev->bus_number,
> +handle->dev->device_address, NULL);

Again the check + call need to happen under the lock.

> continue;
> }
> diff --git a/libusb/os/linux_usbfs.h b/libusb/os/linux_usbfs.h
> index 499bab7..1f5b191 100644
> --- a/libusb/os/linux_usbfs.h
> +++ b/libusb/os/linux_usbfs.h
> @@ -170,7 +170,7 @@ void linux_netlink_hotplug_poll(void);
>   #endif
>   void linux_hotplug_enumerate(uint8_t busnum, uint8_t devaddr, const char 
> *sys_name);
> -void linux_hotplug_disconnected(uint8_t busnum, uint8_t devaddr, const char 
> *sys_name);
> +void linux_device_disconnected(uint8_t busnum, uint8_t devaddr, const char 
> *sys_name);
>   int linux_get_device_address (struct libusb_context *ctx, int detached,
> uint8_t *busnum, uint8_t *devadd

[Libusbx-devel] Improved logging

[Toby Gray]

I've made a couple of changes to the logging in libusbx. The first is
to make a single fprintf call for each log line, as suggested by Chris
Dickens.

The second change makes it possible to enable logging output in
Windows to go to the debugger. This makes it considerably easier to
debug issues in applications without a console. Previously I'd been
making this change locally, but I think it is of general use.

I'd certainly value the input from other Windows developers on the
second of these patches, so it's probably not worth putting this master
until 1.0.16 has been released.

Regards,

Toby

--
This SF.net email is sponsored by Windows:

Build for Windows Store.

http://p.sf.net/sfu/windows-dev2dev
___
libusbx-devel mailing list
libusbx-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libusbx-devel

[Libusbx-devel] [PATCH 2/2] Windows: Make ENABLE_DEBUG_LOGGING enable logging output to the debugger.

[Toby Gray]

This change makes it considerably easier to debug issues in UI applications
which don't necessarily have a console connected to stderr.

Outputting to the debugger shouldn't occur in normal situations so
this change has to be explicitly enabled by a build-time config flag.
---
 libusb/core.c |   10 ++
 1 file changed, 10 insertions(+)

diff --git a/libusb/core.c b/libusb/core.c
index 5c93f4b..33f2a7b 100644
--- a/libusb/core.c
+++ b/libusb/core.c
@@ -2019,6 +2019,16 @@ int usbi_gettimeofday(struct timeval *tp, void *tzp)
 
 static void usbi_log_str(struct libusb_context *ctx, const char * str)
 {
+#ifdef ENABLE_DEBUG_LOGGING
+#if defined(OS_WINDOWS)
+   OutputDebugStringA(str);
+#elif defined(OS_WINCE)
+   /* Windows CE only supports the unicode version of OutputDebugString. */
+   WCHAR wbuf[USBI_MAX_LOG_LEN];
+   MultiByteToWideChar(CP_ACP, 0, str, -1, wbuf, sizeof(wbuf));
+   OutputDebugStringW(wbuf);
+#endif
+#endif /* ENABLE_DEBUG_LOGGING */
UNUSED(ctx);
fprintf(stderr, str);
 }
-- 
1.7.9.5


--
This SF.net email is sponsored by Windows:

Build for Windows Store.

http://p.sf.net/sfu/windows-dev2dev
___
libusbx-devel mailing list
libusbx-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libusbx-devel

[Libusbx-devel] [PATCH 1/2] Core: Make writing of log lines a single fprintf call.

[Toby Gray]

---
 libusb/core.c|   40 +---
 libusb/libusbi.h |   10 ++
 2 files changed, 43 insertions(+), 7 deletions(-)

diff --git a/libusb/core.c b/libusb/core.c
index bc44415..5c93f4b 100644
--- a/libusb/core.c
+++ b/libusb/core.c
@@ -2017,12 +2017,19 @@ int usbi_gettimeofday(struct timeval *tp, void *tzp)
 }
 #endif
 
+static void usbi_log_str(struct libusb_context *ctx, const char * str)
+{
+   UNUSED(ctx);
+   fprintf(stderr, str);
+}
+
 void usbi_log_v(struct libusb_context *ctx, enum libusb_log_level level,
const char *function, const char *format, va_list args)
 {
const char *prefix = "";
+   char buf[USBI_MAX_LOG_LEN];
struct timeval now;
-   int global_debug;
+   int global_debug, header_len, text_len;
static int has_debug_header_been_displayed = 0;
 
 #ifdef ENABLE_DEBUG_LOGGING
@@ -2068,8 +2075,8 @@ void usbi_log_v(struct libusb_context *ctx, enum 
libusb_log_level level,
usbi_gettimeofday(&now, NULL);
if ((global_debug) && (!has_debug_header_been_displayed)) {
has_debug_header_been_displayed = 1;
-   fprintf(stderr, "[timestamp] [threadID] facility level 
[function call] \n");
-   fprintf(stderr, 
"\n");
+   usbi_log_str(ctx, "[timestamp] [threadID] facility level 
[function call] \n");
+   usbi_log_str(ctx, 
"\n");
}
if (now.tv_usec < timestamp_origin.tv_usec) {
now.tv_sec--;
@@ -2099,15 +2106,34 @@ void usbi_log_v(struct libusb_context *ctx, enum 
libusb_log_level level,
}
 
if (global_debug) {
-   fprintf(stderr, "[%2d.%06d] [%08x] libusbx: %s [%s] ",
+   header_len = snprintf(buf, sizeof(buf),
+   "[%2d.%06d] [%08x] libusbx: %s [%s] ",
(int)now.tv_sec, (int)now.tv_usec, usbi_get_tid(), 
prefix, function);
} else {
-   fprintf(stderr, "libusbx: %s [%s] ", prefix, function);
+   header_len = snprintf(buf, sizeof(buf),
+   "libusbx: %s [%s] ", prefix, function);
}
 
-   vfprintf(stderr, format, args);
+   if (header_len < 0 || header_len >= sizeof(buf)) {
+   /* Somehow snprintf failed to write to the buffer,
+* remove the header so something useful is output. */
+   header_len = 0;
+   }
+   text_len = vsnprintf(buf + header_len, sizeof(buf) - header_len,
+   format, args);
+   if (text_len < 0) {
+   text_len = 0;
+   } else if (text_len + header_len >= sizeof(buf)) {
+   /* Truncated log output. */
+   text_len = sizeof(buf) - header_len;
+   }
+   if (header_len + text_len + sizeof(USBI_LOG_LINE_END) >= sizeof(buf)) {
+   /* Need to truncate the text slightly to fit on the terminator. 
*/
+   text_len -= (header_len + text_len + sizeof(USBI_LOG_LINE_END)) 
- sizeof(buf);
+   }
+   strcpy(buf + header_len + text_len, USBI_LOG_LINE_END);
 
-   fprintf(stderr, "\n");
+   usbi_log_str(ctx, buf);
 #endif
 }
 
diff --git a/libusb/libusbi.h b/libusb/libusbi.h
index c9b402d..7293901 100644
--- a/libusb/libusbi.h
+++ b/libusb/libusbi.h
@@ -56,6 +56,11 @@
 #define USBI_CAP_HAS_HID_ACCESS
0x0001
 #define USBI_CAP_SUPPORTS_DETACH_KERNEL_DRIVER 0x0002
 
+/* Maximum number of bytes in a log line */
+#define USBI_MAX_LOG_LEN 256
+/* Terminator for log lines */
+#define USBI_LOG_LINE_END "\n"
+
 /* The following is used to silence warnings for unused variables */
 #define UNUSED(var)do { (void)(var); } while(0)
 
@@ -447,6 +452,11 @@ int usbi_gettimeofday(struct timeval *tp, void *tzp);
 #endif
 #endif
 
+#if (defined(OS_WINDOWS) || defined(OS_WINCE)) && !defined(__GCC__)
+#define snprintf _snprintf
+#define vsnprintf _vsnprintf
+#endif
+
 struct usbi_pollfd {
/* must come first */
struct libusb_pollfd pollfd;
-- 
1.7.9.5


--
This SF.net email is sponsored by Windows:

Build for Windows Store.

http://p.sf.net/sfu/windows-dev2dev
___
libusbx-devel mailing list
libusbx-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libusbx-devel

Re: [Libusbx-devel] [PATCH 1/2] Core: Make writing of log lines a single fprintf call.

[Hans de Goede]

Hi,



A (v)snprintf return of -1 means that the output was truncated
(on some libc-s) throwing away the header if that got truncated
somehow is fine, but throwing away the log message is not.

Thus I would like to suggest replacing:
> + text_len = vsnprintf(buf + header_len, sizeof(buf) - header_len,
> + format, args);
> + if (text_len < 0) {
> + text_len = 0;
> + } else if (text_len + header_len >= sizeof(buf)) {
> + /* Truncated log output. */
> + text_len = sizeof(buf) - header_len;
> + }

With:

 > +text_len = vsnprintf(buf + header_len, sizeof(buf) - header_len,
 > +format, args);
 > +if (text_len < 0 || text_len + header_len >= sizeof(buf)) {
 > +/* Truncated log output. */
 > +text_len = sizeof(buf) - header_len;
 > +}

Regards,

Hans

--
This SF.net email is sponsored by Windows:

Build for Windows Store.

http://p.sf.net/sfu/windows-dev2dev
___
libusbx-devel mailing list
libusbx-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libusbx-devel

Re: [Libusbx-devel] [PATCH 1/2] Core: Make writing of log lines a single fprintf call.

[Toby Gray]

On 27/06/13 13:25, Hans de Goede wrote:
> Hi,
>
> 
>
> A (v)snprintf return of -1 means that the output was truncated
> (on some libc-s) throwing away the header if that got truncated
> somehow is fine, but throwing away the log message is not.

I'd forgotten that some libc-s return -1 meaning truncation.

The problem with that is that some other platforms might use -1 to mean 
parameter errors. For example, in rather contrived situations, Windows 
can return -1 on NULL buffer input.

I see a couple of options:

1) Add a configure test and config.h variable for if -1 means truncation 
or not.

2) Just memset the buffer to zero right at the start and then make the 
change you suggest.

I think the second is the easiest and most sensible option.

Regards,

Toby


--
This SF.net email is sponsored by Windows:

Build for Windows Store.

http://p.sf.net/sfu/windows-dev2dev
___
libusbx-devel mailing list
libusbx-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libusbx-devel

[Libusbx-devel] Improved logging v2

[Toby Gray]

This is a slightly reworked version of my previous logging patches.

I've improved the commit message for the first patch and made the
changes suggested by Hans de Goede relating to return value of
snprintf calls.

Regards,

Toby

--
This SF.net email is sponsored by Windows:

Build for Windows Store.

http://p.sf.net/sfu/windows-dev2dev
___
libusbx-devel mailing list
libusbx-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libusbx-devel

[Libusbx-devel] [PATCH v2 2/2] Windows: Make ENABLE_DEBUG_LOGGING enable logging output to the debugger.

[Toby Gray]

This change makes it considerably easier to debug issues in UI applications
which don't necessarily have a console connected to stderr.

Outputting to the debugger shouldn't occur in normal situations so
this change has to be explicitly enabled by a build-time config flag.
---
 libusb/core.c |   10 ++
 1 file changed, 10 insertions(+)

diff --git a/libusb/core.c b/libusb/core.c
index 53f716f..c567398 100644
--- a/libusb/core.c
+++ b/libusb/core.c
@@ -2019,6 +2019,16 @@ int usbi_gettimeofday(struct timeval *tp, void *tzp)
 
 static void usbi_log_str(struct libusb_context *ctx, const char * str)
 {
+#ifdef ENABLE_DEBUG_LOGGING
+#if defined(OS_WINDOWS)
+   OutputDebugStringA(str);
+#elif defined(OS_WINCE)
+   /* Windows CE only supports the unicode version of OutputDebugString. */
+   WCHAR wbuf[USBI_MAX_LOG_LEN];
+   MultiByteToWideChar(CP_ACP, 0, str, -1, wbuf, sizeof(wbuf));
+   OutputDebugStringW(wbuf);
+#endif
+#endif /* ENABLE_DEBUG_LOGGING */
UNUSED(ctx);
fprintf(stderr, str);
 }
-- 
1.7.9.5


--
This SF.net email is sponsored by Windows:

Build for Windows Store.

http://p.sf.net/sfu/windows-dev2dev
___
libusbx-devel mailing list
libusbx-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libusbx-devel

[Libusbx-devel] [PATCH v2 1/2] Core: Make writing of log lines a single fprintf call.

[Toby Gray]

Prior to this change a single line of logging performing several fprintf.

This change gets all the data for a line to be logged in a single
fprintf call. This reduced the chances of writes from another thread
getting intermixed with a log line.

It also makes it easier to change where logs are output to in the future.
---
 libusb/core.c|   41 ++---
 libusb/libusbi.h |   10 ++
 2 files changed, 44 insertions(+), 7 deletions(-)

diff --git a/libusb/core.c b/libusb/core.c
index bc44415..53f716f 100644
--- a/libusb/core.c
+++ b/libusb/core.c
@@ -2017,12 +2017,19 @@ int usbi_gettimeofday(struct timeval *tp, void *tzp)
 }
 #endif
 
+static void usbi_log_str(struct libusb_context *ctx, const char * str)
+{
+   UNUSED(ctx);
+   fprintf(stderr, str);
+}
+
 void usbi_log_v(struct libusb_context *ctx, enum libusb_log_level level,
const char *function, const char *format, va_list args)
 {
const char *prefix = "";
+   char buf[USBI_MAX_LOG_LEN];
struct timeval now;
-   int global_debug;
+   int global_debug, header_len, text_len;
static int has_debug_header_been_displayed = 0;
 
 #ifdef ENABLE_DEBUG_LOGGING
@@ -2068,8 +2075,8 @@ void usbi_log_v(struct libusb_context *ctx, enum 
libusb_log_level level,
usbi_gettimeofday(&now, NULL);
if ((global_debug) && (!has_debug_header_been_displayed)) {
has_debug_header_been_displayed = 1;
-   fprintf(stderr, "[timestamp] [threadID] facility level 
[function call] \n");
-   fprintf(stderr, 
"\n");
+   usbi_log_str(ctx, "[timestamp] [threadID] facility level 
[function call] \n");
+   usbi_log_str(ctx, 
"\n");
}
if (now.tv_usec < timestamp_origin.tv_usec) {
now.tv_sec--;
@@ -2099,15 +2106,35 @@ void usbi_log_v(struct libusb_context *ctx, enum 
libusb_log_level level,
}
 
if (global_debug) {
-   fprintf(stderr, "[%2d.%06d] [%08x] libusbx: %s [%s] ",
+   header_len = snprintf(buf, sizeof(buf),
+   "[%2d.%06d] [%08x] libusbx: %s [%s] ",
(int)now.tv_sec, (int)now.tv_usec, usbi_get_tid(), 
prefix, function);
} else {
-   fprintf(stderr, "libusbx: %s [%s] ", prefix, function);
+   header_len = snprintf(buf, sizeof(buf),
+   "libusbx: %s [%s] ", prefix, function);
}
 
-   vfprintf(stderr, format, args);
+   if (header_len < 0 || header_len >= sizeof(buf)) {
+   /* Somehow snprintf failed to write to the buffer,
+* remove the header so something useful is output. */
+   header_len = 0;
+   }
+   /* Make sure buffer is NUL terminated */
+   buf[header_len] = '\0';
+   text_len = vsnprintf(buf + header_len, sizeof(buf) - header_len,
+   format, args);
+   if (text_len < 0 || text_len + header_len >= sizeof(buf)) {
+   /* Truncated log output. On some platforms a -1 return value 
means
+* that the output was truncated. */
+   text_len = sizeof(buf) - header_len;
+   }
+   if (header_len + text_len + sizeof(USBI_LOG_LINE_END) >= sizeof(buf)) {
+   /* Need to truncate the text slightly to fit on the terminator. 
*/
+   text_len -= (header_len + text_len + sizeof(USBI_LOG_LINE_END)) 
- sizeof(buf);
+   }
+   strcpy(buf + header_len + text_len, USBI_LOG_LINE_END);
 
-   fprintf(stderr, "\n");
+   usbi_log_str(ctx, buf);
 #endif
 }
 
diff --git a/libusb/libusbi.h b/libusb/libusbi.h
index c9b402d..7293901 100644
--- a/libusb/libusbi.h
+++ b/libusb/libusbi.h
@@ -56,6 +56,11 @@
 #define USBI_CAP_HAS_HID_ACCESS
0x0001
 #define USBI_CAP_SUPPORTS_DETACH_KERNEL_DRIVER 0x0002
 
+/* Maximum number of bytes in a log line */
+#define USBI_MAX_LOG_LEN 256
+/* Terminator for log lines */
+#define USBI_LOG_LINE_END "\n"
+
 /* The following is used to silence warnings for unused variables */
 #define UNUSED(var)do { (void)(var); } while(0)
 
@@ -447,6 +452,11 @@ int usbi_gettimeofday(struct timeval *tp, void *tzp);
 #endif
 #endif
 
+#if (defined(OS_WINDOWS) || defined(OS_WINCE)) && !defined(__GCC__)
+#define snprintf _snprintf
+#define vsnprintf _vsnprintf
+#endif
+
 struct usbi_pollfd {
/* must come first */
struct libusb_pollfd pollfd;
-- 
1.7.9.5


--
This SF.net email is sponsored by Windows:

Build for Windows Store.

http://p.sf.net/sfu/windows-dev2dev
___
libusbx-devel mailing list
libusbx-devel@lists.sourceforge.net
https://lists.so

Re: [Libusbx-devel] [PATCH 1/2] Core: Make writing of log lines a single fprintf call.

[Hans de Goede]

Hi,

On 06/27/2013 03:16 PM, Toby Gray wrote:
> On 27/06/13 13:25, Hans de Goede wrote:
>> Hi,
>>
>> 
>>
>> A (v)snprintf return of -1 means that the output was truncated
>> (on some libc-s) throwing away the header if that got truncated
>> somehow is fine, but throwing away the log message is not.
>
> I'd forgotten that some libc-s return -1 meaning truncation.
>
> The problem with that is that some other platforms might use -1 to mean 
> parameter errors. For example, in rather contrived situations, Windows can 
> return -1 on NULL buffer input.
>
> I see a couple of options:
>
> 1) Add a configure test and config.h variable for if -1 means truncation or 
> not.
>
> 2) Just memset the buffer to zero right at the start and then make the change 
> you suggest.
>
> I think the second is the easiest and most sensible option.

Ack, although I don't really see a need for memset, we will never
pass in a NULL buffer, so we should never get -1.

Regards,

Hans

--
This SF.net email is sponsored by Windows:

Build for Windows Store.

http://p.sf.net/sfu/windows-dev2dev
___
libusbx-devel mailing list
libusbx-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libusbx-devel

Re: [Libusbx-devel] Improved logging v2

[Hans de Goede]

Hi,

On 06/27/2013 03:49 PM, Toby Gray wrote:
>
> This is a slightly reworked version of my previous logging patches.
>
> I've improved the commit message for the first patch and made the
> changes suggested by Hans de Goede relating to return value of
> snprintf calls.

Thanks, I've pushed the 1st patch, as it is indeed very useful
for debugging multi-threaded apps. So it will be in 1.0.16rc2
which I plan to release after I get some feedback from Xiaofan
on the openbsd crash.

For the 2nd patch lets wait for Pete's comments.

Regards,

Hans

--
This SF.net email is sponsored by Windows:

Build for Windows Store.

http://p.sf.net/sfu/windows-dev2dev
___
libusbx-devel mailing list
libusbx-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libusbx-devel

Re: [Libusbx-devel] Improved logging v2

[Toby Gray]

On 27/06/13 16:15, Hans de Goede wrote:

Hi,

On 06/27/2013 03:49 PM, Toby Gray wrote:


This is a slightly reworked version of my previous logging patches.

I've improved the commit message for the first patch and made the
changes suggested by Hans de Goede relating to return value of
snprintf calls.


Thanks, I've pushed the 1st patch, as it is indeed very useful
for debugging multi-threaded apps. So it will be in 1.0.16rc2
which I plan to release after I get some feedback from Xiaofan
on the openbsd crash.


Excellent. I've just discovered that it triggers a build warning. I've 
attached a fix for this build warning.




For the 2nd patch lets wait for Pete's comments.


Agreed.

Regards,

Toby
>From 60ce7c013a99b054337db0407731840a0b6c8b2a Mon Sep 17 00:00:00 2001
From: Toby Gray 
Date: Thu, 27 Jun 2013 16:47:46 +0100
Subject: [PATCH] Core: Fix warning in use of fprintf.

Some versions of GCC can check that format strings are being used
securely. This commit fixes the following warning:

  core.c:2023:2: warning: format not a string literal and no format arguments [-Wformat-security]
---
 libusb/core.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libusb/core.c b/libusb/core.c
index 53f716f..ec37ba1 100644
--- a/libusb/core.c
+++ b/libusb/core.c
@@ -2020,7 +2020,7 @@ int usbi_gettimeofday(struct timeval *tp, void *tzp)
 static void usbi_log_str(struct libusb_context *ctx, const char * str)
 {
 	UNUSED(ctx);
-	fprintf(stderr, str);
+	fprintf(stderr, "%s", str);
 }
 
 void usbi_log_v(struct libusb_context *ctx, enum libusb_log_level level,
-- 
1.7.9.5

--
This SF.net email is sponsored by Windows:

Build for Windows Store.

http://p.sf.net/sfu/windows-dev2dev___
libusbx-devel mailing list
libusbx-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libusbx-devel

Re: [Libusbx-devel] Improving linux hotplug disconnect

[Chris Dickens]

Hi,


On Thu, Jun 27, 2013 at 3:31 AM, Hans de Goede  wrote:

> Hi,
>
>  +/* device will still be marked as attached if hotplug monitor thread
>> +* hasn't processed remove event yet */
>> +if (handle->dev->attached) {
>>
>
> This check is racy, you need to take the hotplug lock before making this
> check, also linux_device_disconnected expects the hotplug lock to be
> taken when it is called, so simply but a check around the entire block.


Ok, patch revised. With the current design this race condition is benign,
but better to be safe than sorry if something changes in the future.


> Ok, that makes sense, I can live with having a check in 2 places, please
> respin with the suggested
> fixes and send with git send-email, or attach, so that your mail client
> does not foobar the patch.
>

Patch is attached.

Regards,
Chris


0001-linux-Handle-device-disconnection-early-when-possibl.patch
Description: Binary data
--
This SF.net email is sponsored by Windows:

Build for Windows Store.

http://p.sf.net/sfu/windows-dev2dev___
libusbx-devel mailing list
libusbx-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libusbx-devel

[Libusbx-devel] [PATCH 10/11] Windows: Pre-reserve pollfds before submitting transfers.

[Toby Gray]

This change makes the Windows and Windows CE backend pre-reserve
pollfds before submitting transfers. This allows these backends to
correctly handle failure to reserve pollfds.

Prior to this change both backends ignored the return value from
usbi_add_pollfds.
---
 libusb/os/wince_usb.c   |   13 -
 libusb/os/windows_usb.c |   35 ++-
 2 files changed, 42 insertions(+), 6 deletions(-)

diff --git a/libusb/os/wince_usb.c b/libusb/os/wince_usb.c
index 9957b8e..d1c3432 100644
--- a/libusb/os/wince_usb.c
+++ b/libusb/os/wince_usb.c
@@ -618,6 +618,13 @@ static int wince_submit_control_or_bulk_transfer(struct 
usbi_transfer *itransfer
HANDLE eventHandle;
PUKW_CONTROL_HEADER setup = NULL;
const BOOL control_transfer = transfer->type == 
LIBUSB_TRANSFER_TYPE_CONTROL;
+   struct usbi_pollfd* ipollfd;
+
+   ret = usbi_reserve_pollfd(ctx, &ipollfd);
+   if (ret != LIBUSB_SUCCESS) {
+   usbi_err(ctx, "Failed to reserve pollfd for async transfer");
+   return ret;
+   }
 
transfer_priv->pollable_fd = INVALID_WINFD;
if (control_transfer) {
@@ -632,12 +639,14 @@ static int wince_submit_control_or_bulk_transfer(struct 
usbi_transfer *itransfer
eventHandle = CreateEvent(NULL, FALSE, FALSE, NULL);
if (eventHandle == NULL) {
usbi_err(ctx, "Failed to create event for async transfer");
+   usbi_unreserve_pollfd(ctx, ipollfd);
return LIBUSB_ERROR_NO_MEM;
}
 
wfd = usbi_create_fd(eventHandle, direction_in ? RW_READ : RW_WRITE, 
itransfer, &wince_cancel_transfer);
if (wfd.handle == INVALID_HANDLE_VALUE) {
CloseHandle(eventHandle);
+   usbi_unreserve_pollfd(ctx, ipollfd);
return LIBUSB_ERROR_NO_MEM;
}
 
@@ -657,9 +666,11 @@ static int wince_submit_control_or_bulk_transfer(struct 
usbi_transfer *itransfer
usbi_err(ctx, "UkwIssue%sTransfer failed: error %d",
control_transfer ? "Control" : "Bulk", GetLastError());
wince_clear_transfer_priv(itransfer);
+   usbi_unreserve_pollfd(ctx, ipollfd);
return libusbErr;
}
-   usbi_add_pollfd(ctx, transfer_priv->pollable_fd.overlapped->hEvent, 
direction_in ? POLLIN : POLLOUT);
+   usbi_add_reserved_pollfd(ctx, 
transfer_priv->pollable_fd.overlapped->hEvent,
+   direction_in ? POLLIN : POLLOUT, ipollfd);
itransfer->flags |= USBI_TRANSFER_UPDATED_FDS;
 
return LIBUSB_SUCCESS;
diff --git a/libusb/os/windows_usb.c b/libusb/os/windows_usb.c
index 26cb7f8..a9090e4 100644
--- a/libusb/os/windows_usb.c
+++ b/libusb/os/windows_usb.c
@@ -1926,14 +1926,22 @@ static int submit_bulk_transfer(struct usbi_transfer 
*itransfer)
struct windows_transfer_priv *transfer_priv = (struct 
windows_transfer_priv*)usbi_transfer_get_os_priv(itransfer);
struct windows_device_priv *priv = 
_device_priv(transfer->dev_handle->dev);
int r;
+   struct usbi_pollfd* ipollfd;
+
+   r = usbi_reserve_pollfd(ctx, &ipollfd);
+   if (r != LIBUSB_SUCCESS) {
+   usbi_err(ctx, "Failed to reserve pollfd for async transfer");
+   return r;
+   }
 
r = priv->apib->submit_bulk_transfer(SUB_API_NOTSET, itransfer);
if (r != LIBUSB_SUCCESS) {
+   usbi_unreserve_pollfd(ctx, ipollfd);
return r;
}
 
-   usbi_add_pollfd(ctx, transfer_priv->pollable_fd.overlapped->hEvent,
-   (short)(IS_XFERIN(transfer) ? POLLIN : POLLOUT));
+   usbi_add_reserved_pollfd(ctx, 
transfer_priv->pollable_fd.overlapped->hEvent,
+   (short)(IS_XFERIN(transfer) ? POLLIN : POLLOUT), ipollfd);
 
itransfer->flags |= USBI_TRANSFER_UPDATED_FDS;
return LIBUSB_SUCCESS;
@@ -1946,14 +1954,22 @@ static int submit_iso_transfer(struct usbi_transfer 
*itransfer)
struct windows_transfer_priv *transfer_priv = (struct 
windows_transfer_priv*)usbi_transfer_get_os_priv(itransfer);
struct windows_device_priv *priv = 
_device_priv(transfer->dev_handle->dev);
int r;
+   struct usbi_pollfd* ipollfd;
+
+   r = usbi_reserve_pollfd(ctx, &ipollfd);
+   if (r != LIBUSB_SUCCESS) {
+   usbi_err(ctx, "Failed to reserve pollfd for async transfer");
+   return r;
+   }
 
r = priv->apib->submit_iso_transfer(SUB_API_NOTSET, itransfer);
if (r != LIBUSB_SUCCESS) {
+   usbi_unreserve_pollfd(ctx, ipollfd);
return r;
}
 
-   usbi_add_pollfd(ctx, transfer_priv->pollable_fd.overlapped->hEvent,
-   (short)(IS_XFERIN(transfer) ? POLLIN : POLLOUT));
+   usbi_add_reserved_pollfd(ctx, 
transfer_priv->pollable_fd.overlapped->hEvent,
+   (short)(IS_XFERIN(transfer) ? POLLIN : POLLOUT), ipollfd);
 
itransfer->flags |= USBI_TRANSF

[Libusbx-devel] [PATCH 07/11] Tests: Add test for 64 concurrent control requests.

[Toby Gray]

This test is being added to provoke MAXIMUM_WAIT_OBJECTS on Windows
platforms.

Currently the synchronous transfer in this test hangs on Windows
platforms. This is due to the following sequence of events:

* There are more than MAXIMUM_WAIT_OBJECTS event objects to wait for.
* WaitForMultipleObjects fails with an error.
* usbi_poll detects this error and returns it.
* sync_transfer_wait_for_completion() receives this error and cancels
  the transfer.
* The transfer cancel never comes through as usbi_poll still has too many
  event objects.
---
 tests/libusbx_testlib.h |   17 +
 tests/stress.c  |   96 +++
 tests/testlib.c |   11 ++
 3 files changed, 124 insertions(+)

diff --git a/tests/libusbx_testlib.h b/tests/libusbx_testlib.h
index 8b109a2..678ff86 100644
--- a/tests/libusbx_testlib.h
+++ b/tests/libusbx_testlib.h
@@ -158,4 +158,21 @@ libusb_context * 
libusbx_testlib_init_ctx_or_fail(libusbx_testlib_ctx * tctx);
 libusb_device_handle * libusbx_testlib_open_test_device(
libusbx_testlib_ctx * tctx, libusb_context * ctx);
 
+/**
+ * Fails the test with the given reason.
+ */
+void libusbx_testlib_fail(libusbx_testlib_ctx * tctx,
+   const char * file,
+   int line,
+   const char * reason);
+
+/**
+ * Helper macro to make calling libusbx_testlib_assert() easier.
+ */
+#define LIBUSBX_TEST_ASSERT(tctx, statement) \
+do { \
+   if (!(statement)) \
+   libusbx_testlib_fail((tctx), __FILE__, __LINE__, #statement); \
+} while (0)
+
 #endif //LIBUSBX_TESTLIB_H
diff --git a/tests/stress.c b/tests/stress.c
index f64f4b7..c0868f6 100644
--- a/tests/stress.c
+++ b/tests/stress.c
@@ -144,12 +144,108 @@ static libusbx_testlib_result 
test_default_context_change(libusbx_testlib_ctx *
return TEST_STATUS_SUCCESS;
 }
 
+void LIBUSB_CALL transfer_cb(struct libusb_transfer * transfer)
+{
+   int * done = (int*) transfer->user_data;
+   *done = 1;
+}
+
+#define GET_MANY_CFG_COUNT 64
+/** Tests many concurrent getting device configuration control requests. */
+static libusbx_testlib_result test_many_get_configuration_requests(
+   libusbx_testlib_ctx * tctx)
+{
+   struct libusb_transfer* transfers[GET_MANY_CFG_COUNT];
+   unsigned char 
transfer_data[GET_MANY_CFG_COUNT][LIBUSB_CONTROL_SETUP_SIZE];
+   int transfer_done[GET_MANY_CFG_COUNT];
+   unsigned char data[1];
+   int i, r, all_done;
+   libusb_context * ctx = NULL;
+   libusb_device_handle * dev = NULL;
+
+   libusbx_testlib_skip_if_no_device(tctx);
+   ctx = libusbx_testlib_init_ctx_or_fail(tctx);
+   dev = libusbx_testlib_open_test_device(tctx, ctx);
+
+   memset(transfers, 0, sizeof(transfers));
+   memset(transfer_data, 0, sizeof(transfer_data));
+   memset(transfer_done, 0, sizeof(transfer_done));
+
+   /* Issue all the control transfers */
+   for(i = 0; i < GET_MANY_CFG_COUNT; ++i) {
+   transfers[i] = libusb_alloc_transfer(0);
+   LIBUSBX_TEST_ASSERT(tctx, transfers[i] != NULL);
+
+   libusb_fill_control_setup(transfer_data[i],
+   LIBUSB_ENDPOINT_IN |
+   LIBUSB_REQUEST_TYPE_STANDARD |
+   LIBUSB_RECIPIENT_DEVICE,
+   LIBUSB_REQUEST_GET_CONFIGURATION,
+   0, 0, 1);
+   libusb_fill_control_transfer(transfers[i],
+   dev, transfer_data[i],
+   &transfer_cb, &transfer_done[i],
+   6);
+
+   r = libusb_submit_transfer(transfers[i]);
+   LIBUSBX_TEST_ASSERT(tctx, r == LIBUSB_SUCCESS || r == 
LIBUSB_ERROR_NOT_SUPPORTED);
+   if (r == LIBUSB_ERROR_NOT_SUPPORTED) {
+   /* All platforms should support at least 16 concurrent 
transfers */
+   LIBUSBX_TEST_ASSERT(tctx, i > 16);
+   /* Mark this request as already done */
+   transfer_done[i] = 1;
+   }
+   }
+
+   /* Issue a synchronous control transfer, this tests the error handling
+* in the synchronous case. */
+   r = libusb_control_transfer(dev,
+   LIBUSB_ENDPOINT_IN |
+   LIBUSB_REQUEST_TYPE_STANDARD |
+   LIBUSB_RECIPIENT_DEVICE,
+   LIBUSB_REQUEST_GET_CONFIGURATION,
+   0,
+   0,
+   data, sizeof(data), 1000);
+   LIBUSBX_TEST_ASSERT(tctx, r == 1 ||
+   r == LIBUSB_SUCCESS || r == LIBUSB_ERROR_NOT_SUPPORTED);
+
+   /* Process events until all the transfers are finished.
+* This isn't done in a very sensible way as it just polls
+* every 250ms to see if all the transfers have completed. */
+   do {
+   struct timeval tv;
+   tv.tv_sec = 0;
+   tv.tv_usec = 25000

[Libusbx-devel] [PATCH 08/11] Core: Cache the number of pollfds.

[Toby Gray]

Previously it was necessary to count the number of pollfds contained
in a linked list. This change introduces the optimisation of storing a
running total of the number of pollfds.
---
 libusb/io.c  |   16 
 libusb/libusbi.h |1 +
 2 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/libusb/io.c b/libusb/io.c
index 314184f..d6eb3b5 100644
--- a/libusb/io.c
+++ b/libusb/io.c
@@ -1060,6 +1060,7 @@ int usbi_io_init(struct libusb_context *ctx)
usbi_cond_init(&ctx->event_waiters_cond, NULL);
list_init(&ctx->flying_transfers);
list_init(&ctx->pollfds);
+   ctx->pollfds_cnt = 0;
 
/* FIXME should use an eventfd on kernels that support it */
r = usbi_pipe(ctx->ctrl_pipe);
@@ -1893,8 +1894,7 @@ static int handle_events(struct libusb_context *ctx, 
struct timeval *tv)
int timeout_ms;
 
usbi_mutex_lock(&ctx->pollfds_lock);
-   list_for_each_entry(ipollfd, &ctx->pollfds, list, struct usbi_pollfd)
-   nfds++;
+   nfds = ctx->pollfds_cnt;
 
/* TODO: malloc when number of fd's changes, not on every poll */
if (nfds != 0)
@@ -2421,6 +2421,7 @@ int usbi_add_pollfd(struct libusb_context *ctx, 
libusb_event_handle handle, shor
ipollfd->pollfd.direction = poll_events_to_event_dirs(events);
usbi_mutex_lock(&ctx->pollfds_lock);
list_add_tail(&ipollfd->list, &ctx->pollfds);
+   ++ctx->pollfds_cnt;
usbi_mutex_unlock(&ctx->pollfds_lock);
 
 #ifdef LIBUSB_EVENT_HANDLE_IS_FD
@@ -2453,6 +2454,7 @@ void usbi_remove_pollfd(struct libusb_context *ctx, 
libusb_event_handle handle)
}
 
list_del(&ipollfd->list);
+   --ctx->pollfds_cnt;
usbi_mutex_unlock(&ctx->pollfds_lock);
free(ipollfd);
 #ifdef LIBUSB_EVENT_HANDLE_IS_FD
@@ -2492,9 +2494,8 @@ const struct libusb_pollfd ** LIBUSB_CALL 
libusb_get_pollfds(
USBI_GET_CONTEXT(ctx);
 
usbi_mutex_lock(&ctx->pollfds_lock);
-   list_for_each_entry(ipollfd, &ctx->pollfds, list, struct usbi_pollfd)
-   cnt++;
-
+   cnt = ctx->pollfds_cnt;
+   
/* Allocate enough space for the pointers and the libusb_event
 * structures after the last pointer. */
alloc_size = ((cnt + 1) * sizeof(struct libusb_pollfd *)) +
@@ -2553,9 +2554,8 @@ struct libusb_event ** LIBUSB_CALL libusb_get_events(
USBI_GET_CONTEXT(ctx);
 
usbi_mutex_lock(&ctx->pollfds_lock);
-   list_for_each_entry(ipollfd, &ctx->pollfds, list, struct usbi_pollfd)
-   cnt++;
-
+   cnt = ctx->pollfds_cnt;
+   
/* Allocate enough space for the pointers and the libusb_event
 * structures after the last pointer. */
alloc_size = ((cnt + 1) * sizeof(struct libusb_event *)) +
diff --git a/libusb/libusbi.h b/libusb/libusbi.h
index 7c4f1f1..52d39ff 100644
--- a/libusb/libusbi.h
+++ b/libusb/libusbi.h
@@ -247,6 +247,7 @@ struct libusb_context {
 
/* list of poll fds */
struct list_head pollfds;
+   size_t pollfds_cnt;
usbi_mutex_t pollfds_lock;
 
/* a counter that is set when we want to interrupt event handling, in 
order
-- 
1.7.9.5


--
This SF.net email is sponsored by Windows:

Build for Windows Store.

http://p.sf.net/sfu/windows-dev2dev
___
libusbx-devel mailing list
libusbx-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libusbx-devel

[Libusbx-devel] [PATCH 03/11] Core: Add new cross-platform event handling API

[Toby Gray]

This change adds the API and some stub implementations of a new
cross platform supporting event API.
---
 libusb/io.c   |   78 -
 libusb/libusb-1.0.def |4 +++
 libusb/libusb.h   |   65 +
 libusb/libusbi.h  |9 --
 4 files changed, 141 insertions(+), 15 deletions(-)

diff --git a/libusb/io.c b/libusb/io.c
index b27e082..67c0d8b 100644
--- a/libusb/io.c
+++ b/libusb/io.c
@@ -2356,43 +2356,75 @@ void API_EXPORTED 
libusb_set_pollfd_notifiers(libusb_context *ctx,
ctx->fd_cb_user_data = user_data;
 }
 
-/* Add a file descriptor to the list of file descriptors to be monitored.
- * events should be specified as a bitmask of events passed to poll(), e.g.
- * POLLIN and/or POLLOUT. */
-int usbi_add_pollfd(struct libusb_context *ctx, int fd, short events)
+/** \ingroup events
+ * Register notification functions for event object additions/removals.
+ * These functions will be invoked for every new or removed file descriptor
+ * that libusbx uses as an event source.
+ *
+ * To remove notifiers, pass NULL values for the function pointers.
+ *
+ * Note that events may have been added even before you register
+ * these notifiers (e.g. at libusb_init() time).
+ *
+ * Additionally, note that the removal notifier may be called during
+ * libusb_exit() (e.g. when it is closing file descriptors that were opened
+ * and added to the poll set at libusb_init() time). If you don't want this,
+ * remove the notifiers immediately before calling libusb_exit().
+ *
+ * \param ctx the context to operate on, or NULL for the default context
+ * \param added_cb pointer to function for addition notifications
+ * \param removed_cb pointer to function for removal notifications
+ * \param user_data User data to be passed back to callbacks (useful for
+ * passing context information)
+ */
+void API_EXPORTED libusb_set_event_notifiers(libusb_context *ctx,
+   libusb_event_added_cb added_cb, libusb_event_removed_cb removed_cb,
+   void *user_data)
+{
+   USBI_GET_CONTEXT(ctx);
+   ctx->event_added_cb = added_cb;
+   ctx->event_removed_cb = removed_cb;
+   ctx->event_cb_user_data = user_data;
+}
+
+/* Add an event handle to the list of event handles to be monitored.
+ * events should be specified as a bitmask of libusb_event_direction. */
+int usbi_add_event_handle(struct libusb_context *ctx, libusb_event_handle 
handle, short events)
 {
struct usbi_pollfd *ipollfd = malloc(sizeof(*ipollfd));
if (!ipollfd)
return LIBUSB_ERROR_NO_MEM;
 
-   usbi_dbg("add fd %d events %d", fd, events);
-   ipollfd->pollfd.fd = fd;
+   usbi_dbg("add handle %d events %d", handle, events);
+   ipollfd->pollfd.fd = handle;
ipollfd->pollfd.events = events;
usbi_mutex_lock(&ctx->pollfds_lock);
list_add_tail(&ipollfd->list, &ctx->pollfds);
usbi_mutex_unlock(&ctx->pollfds_lock);
 
if (ctx->fd_added_cb)
-   ctx->fd_added_cb(fd, events, ctx->fd_cb_user_data);
+   ctx->fd_added_cb(handle, events, ctx->fd_cb_user_data);
+   if (ctx->event_added_cb)
+   ctx->event_added_cb(handle, events, ctx->event_cb_user_data);
return 0;
 }
 
-/* Remove a file descriptor from the list of file descriptors to be polled. */
-void usbi_remove_pollfd(struct libusb_context *ctx, int fd)
+/* Remove an event handle from the list of event handles to be polled. */
+void usbi_remove_event_handle(struct libusb_context *ctx, libusb_event_handle 
handle)
 {
struct usbi_pollfd *ipollfd;
int found = 0;
 
-   usbi_dbg("remove fd %d", fd);
+   usbi_dbg("remove handle %d", handle);
usbi_mutex_lock(&ctx->pollfds_lock);
list_for_each_entry(ipollfd, &ctx->pollfds, list, struct usbi_pollfd)
-   if (ipollfd->pollfd.fd == fd) {
+   if (ipollfd->pollfd.fd == handle) {
found = 1;
break;
}
 
if (!found) {
-   usbi_dbg("couldn't find fd %d to remove", fd);
+   usbi_dbg("couldn't find fd %d to remove", handle);
usbi_mutex_unlock(&ctx->pollfds_lock);
return;
}
@@ -2401,7 +2433,9 @@ void usbi_remove_pollfd(struct libusb_context *ctx, int 
fd)
usbi_mutex_unlock(&ctx->pollfds_lock);
free(ipollfd);
if (ctx->fd_removed_cb)
-   ctx->fd_removed_cb(fd, ctx->fd_cb_user_data);
+   ctx->fd_removed_cb(handle, ctx->fd_cb_user_data);
+   if (ctx->event_removed_cb)
+   ctx->event_removed_cb(handle, ctx->event_cb_user_data);
 }
 
 /** \ingroup poll
@@ -2452,6 +2486,24 @@ out:
 #endif
 }
 
+/** \ingroup events
+ * Retrieve a list of events that should be polled by your main loop
+ * as libusbx event sources.
+ *
+ * The returned list is NULL-terminated and should be freed with free() when
+ * done. Th

[Libusbx-devel] [PATCH 05/11] Windows: Change Windows poll code to use libusb_event_handle

[Toby Gray]

This updates the common poll code used by Windows and Windows CE to
use the libusb_event_handle typedef instead of int for file
descriptors.
---
 libusb/os/poll_windows.c |8 
 libusb/os/poll_windows.h |   10 +-
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/libusb/os/poll_windows.c b/libusb/os/poll_windows.c
index 7ed19ba..6d4f934 100644
--- a/libusb/os/poll_windows.c
+++ b/libusb/os/poll_windows.c
@@ -256,7 +256,7 @@ void exit_polling(void)
  * event. To that extent, we create a single wfd and overlapped as a means
  * to access that event.
  */
-int usbi_pipe(int filedes[2])
+int usbi_pipe(libusb_event_handle filedes[2])
 {
int i;
OVERLAPPED* overlapped;
@@ -626,7 +626,7 @@ poll_exit:
 /*
  * close a fake pipe fd
  */
-int usbi_close(int fd)
+int usbi_close(libusb_event_handle fd)
 {
int _index;
int r = -1;
@@ -648,7 +648,7 @@ int usbi_close(int fd)
 /*
  * synchronous write for fake "pipe" signaling
  */
-ssize_t usbi_write(int fd, const void *buf, size_t count)
+ssize_t usbi_write(libusb_event_handle fd, const void *buf, size_t count)
 {
int _index;
UNUSED(buf);
@@ -684,7 +684,7 @@ ssize_t usbi_write(int fd, const void *buf, size_t count)
 /*
  * synchronous read for fake "pipe" signaling
  */
-ssize_t usbi_read(int fd, void *buf, size_t count)
+ssize_t usbi_read(libusb_event_handle fd, void *buf, size_t count)
 {
int _index;
ssize_t r = -1;
diff --git a/libusb/os/poll_windows.h b/libusb/os/poll_windows.h
index deed206..b52031b 100644
--- a/libusb/os/poll_windows.h
+++ b/libusb/os/poll_windows.h
@@ -59,7 +59,7 @@ extern enum windows_version windows_version;
 #define POLLNVAL0x0020/* Invalid request: fd not open */
 
 struct pollfd {
-int fd;   /* file descriptor */
+libusb_event_handle fd;   /* file descriptor */
 short events; /* requested events */
 short revents;/* returned events */
 };
@@ -84,11 +84,11 @@ struct winfd {
 };
 extern const struct winfd INVALID_WINFD;
 
-int usbi_pipe(int pipefd[2]);
+int usbi_pipe(libusb_event_handle pipefd[2]);
 int usbi_poll(struct pollfd *fds, unsigned int nfds, int timeout);
-ssize_t usbi_write(int fd, const void *buf, size_t count);
-ssize_t usbi_read(int fd, void *buf, size_t count);
-int usbi_close(int fd);
+ssize_t usbi_write(libusb_event_handle fd, const void *buf, size_t count);
+ssize_t usbi_read(libusb_event_handle fd, void *buf, size_t count);
+int usbi_close(libusb_event_handle fd);
 
 void init_polling(void);
 void exit_polling(void);
-- 
1.7.9.5


--
This SF.net email is sponsored by Windows:

Build for Windows Store.

http://p.sf.net/sfu/windows-dev2dev
___
libusbx-devel mailing list
libusbx-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libusbx-devel

[Libusbx-devel] [RFC] Remove fake fds from Windows backends

[Toby Gray]

I realise that 1.0.16 is currently trying to be released and that
these patches won't get looked at until after that has happened. I'm
away from the office for the next couple of weeks so thought it was
worth sending this patch series for people to look at if they get
time while I'm away.

A while ago I mentioned that I thought that I was seeing performance
issues due to all the fake FD logic in the Windows backends. This patch
series makes these changes.

This series is actually a group of related changes.

Patches 1, 2 and 7 add some additional tests and testlib functionality.

Patch 3 adds a new event API which provides FDs on POSIX systems but
HANDLES on Windows systems.

Patch 4 changes the core code to make use of the new FD/HANDLE type.

Patch 5 and 6 change the Windows backends over to using the FD/HANDLE type.

While developing these patches I noticed that the Windows backends have a bug
where if you get more than MAXIMUM_WAIT_OBJECTS pollfds then any attempt at
a synchronous transfer will hang. The problem is that the handling of
events starts to fail when too many transfers have been submitted. This
is impossible to recover from as event handling needs to occur to remove
the pollfds for cancelled/completed transfers.

Patches 8 and 9 modify the core code to allow pre-reserving of pollfds
and limiting the total number of concurrent pollfds. Instead of failing
in handling events with high numbers of pollfds this makes it possible
to fail the submission of the transfer.

Patch 10 makes use of pre-reserving of pollfds in the Windows backends.

Finally patch 11 enables limiting the number of concurrent pollfds on
Windows platforms.

As for the original reason for this patch series, our actual
performance uses cases have improved by about 5% on WinCE through
these changes.

Sadly the simple performance tests I created don't show any improvement
as they are limited by IO scheduling, not CPU usage.

Regards,

Toby

--
This SF.net email is sponsored by Windows:

Build for Windows Store.

http://p.sf.net/sfu/windows-dev2dev
___
libusbx-devel mailing list
libusbx-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libusbx-devel

[Libusbx-devel] [PATCH 04/11] Core: Change pollfds over to using libusb_event_handle instead of int

[Toby Gray]

This change makes the internals of libusb make use of the
libusb_event_handle typedef instead of int when dealing with file
descriptors.
---
 libusb/io.c  |  123 +++---
 libusb/libusb.h  |5 ++-
 libusb/libusbi.h |   10 ++---
 3 files changed, 116 insertions(+), 22 deletions(-)

diff --git a/libusb/io.c b/libusb/io.c
index 67c0d8b..314184f 100644
--- a/libusb/io.c
+++ b/libusb/io.c
@@ -1860,6 +1860,27 @@ out:
 }
 #endif
 
+/* Maps the bitmaps in libusb_event_direction into poll() directions */
+static int event_dirs_to_poll_events(enum libusb_event_direction dir)
+{
+   int ret = 0;
+   if ((dir & LIBUSB_EVENT_DIR_IN) == LIBUSB_EVENT_DIR_IN)
+   ret |= POLLIN;
+   if ((dir & LIBUSB_EVENT_DIR_OUT) == LIBUSB_EVENT_DIR_OUT)
+   ret |= POLLOUT;
+   return ret;
+}
+/* Maps the bitmaps in libusb_event_direction into poll() directions */
+static enum libusb_event_direction poll_events_to_event_dirs(short events)
+{
+   enum libusb_event_direction ret = 0;
+   if ((events & POLLIN) == POLLIN)
+   ret |= LIBUSB_EVENT_DIR_IN;
+   if ((events & POLLOUT) == POLLOUT)
+   ret |= LIBUSB_EVENT_DIR_OUT;
+   return ret;
+}
+
 /* do the actual event handling. assumes that no other thread is concurrently
  * doing the same thing. */
 static int handle_events(struct libusb_context *ctx, struct timeval *tv)
@@ -1884,11 +1905,11 @@ static int handle_events(struct libusb_context *ctx, 
struct timeval *tv)
}
 
list_for_each_entry(ipollfd, &ctx->pollfds, list, struct usbi_pollfd) {
-   struct libusb_pollfd *pollfd = &ipollfd->pollfd;
-   int fd = pollfd->fd;
+   struct libusb_event *pollfd = &ipollfd->pollfd;
+   libusb_event_handle fd = pollfd->handle;
i++;
fds[i].fd = fd;
-   fds[i].events = pollfd->events;
+   fds[i].events = event_dirs_to_poll_events(pollfd->direction);
fds[i].revents = 0;
}
usbi_mutex_unlock(&ctx->pollfds_lock);
@@ -2389,28 +2410,30 @@ void API_EXPORTED 
libusb_set_event_notifiers(libusb_context *ctx,
 
 /* Add an event handle to the list of event handles to be monitored.
  * events should be specified as a bitmask of libusb_event_direction. */
-int usbi_add_event_handle(struct libusb_context *ctx, libusb_event_handle 
handle, short events)
+int usbi_add_pollfd(struct libusb_context *ctx, libusb_event_handle handle, 
short events)
 {
struct usbi_pollfd *ipollfd = malloc(sizeof(*ipollfd));
if (!ipollfd)
return LIBUSB_ERROR_NO_MEM;
 
usbi_dbg("add handle %d events %d", handle, events);
-   ipollfd->pollfd.fd = handle;
-   ipollfd->pollfd.events = events;
+   ipollfd->pollfd.handle = handle;
+   ipollfd->pollfd.direction = poll_events_to_event_dirs(events);
usbi_mutex_lock(&ctx->pollfds_lock);
list_add_tail(&ipollfd->list, &ctx->pollfds);
usbi_mutex_unlock(&ctx->pollfds_lock);
 
+#ifdef LIBUSB_EVENT_HANDLE_IS_FD
if (ctx->fd_added_cb)
ctx->fd_added_cb(handle, events, ctx->fd_cb_user_data);
+#endif
if (ctx->event_added_cb)
ctx->event_added_cb(handle, events, ctx->event_cb_user_data);
return 0;
 }
 
 /* Remove an event handle from the list of event handles to be polled. */
-void usbi_remove_event_handle(struct libusb_context *ctx, libusb_event_handle 
handle)
+void usbi_remove_pollfd(struct libusb_context *ctx, libusb_event_handle handle)
 {
struct usbi_pollfd *ipollfd;
int found = 0;
@@ -2418,7 +2441,7 @@ void usbi_remove_event_handle(struct libusb_context *ctx, 
libusb_event_handle ha
usbi_dbg("remove handle %d", handle);
usbi_mutex_lock(&ctx->pollfds_lock);
list_for_each_entry(ipollfd, &ctx->pollfds, list, struct usbi_pollfd)
-   if (ipollfd->pollfd.fd == handle) {
+   if (ipollfd->pollfd.handle == handle) {
found = 1;
break;
}
@@ -2432,8 +2455,10 @@ void usbi_remove_event_handle(struct libusb_context 
*ctx, libusb_event_handle ha
list_del(&ipollfd->list);
usbi_mutex_unlock(&ctx->pollfds_lock);
free(ipollfd);
+#ifdef LIBUSB_EVENT_HANDLE_IS_FD
if (ctx->fd_removed_cb)
ctx->fd_removed_cb(handle, ctx->fd_cb_user_data);
+#endif
if (ctx->event_removed_cb)
ctx->event_removed_cb(handle, ctx->event_cb_user_data);
 }
@@ -2457,23 +2482,41 @@ DEFAULT_VISIBILITY
 const struct libusb_pollfd ** LIBUSB_CALL libusb_get_pollfds(
libusb_context *ctx)
 {
-#ifndef OS_WINDOWS
+#ifdef LIBUSB_EVENT_HANDLE_IS_FD
struct libusb_pollfd **ret = NULL;
+   struct libusb_pollfd * current = NULL;
struct usbi_pollfd *ipollfd;
size_t i = 0;
size_t cnt = 0;
+   size_t alloc_size = 0;
  

[Libusbx-devel] [PATCH 06/11] Windows: Get rid of fake FDs use inside Windows and Window CE backends

[Toby Gray]

This change removes the fake FDs and replaces them with event handles.
---
 libusb/os/poll_windows.c |  466 --
 libusb/os/poll_windows.h |   10 +-
 libusb/os/wince_usb.c|   14 +-
 libusb/os/windows_usb.c  |   53 --
 4 files changed, 118 insertions(+), 425 deletions(-)

diff --git a/libusb/os/poll_windows.c b/libusb/os/poll_windows.c
index 6d4f934..5a96111 100644
--- a/libusb/os/poll_windows.c
+++ b/libusb/os/poll_windows.c
@@ -67,15 +67,8 @@
 #define CHECK_INIT_POLLING do {if(!is_polling_set) init_polling();} while(0)
 
 // public fd data
-const struct winfd INVALID_WINFD = {-1, INVALID_HANDLE_VALUE, NULL, NULL, 
NULL, RW_NONE};
-struct winfd poll_fd[MAX_FDS];
-// internal fd data
-struct {
-   CRITICAL_SECTION mutex; // lock for fds
-   // Additional variables for XP CancelIoEx partial emulation
-   HANDLE original_handle;
-   DWORD thread_id;
-} _poll_fd[MAX_FDS];
+const struct winfd INVALID_WINFD = {INVALID_HANDLE_VALUE, NULL, NULL, NULL, 
RW_NONE,
+   INVALID_HANDLE_VALUE, 0};
 
 // globals
 BOOLEAN is_polling_set = FALSE;
@@ -100,26 +93,22 @@ static inline void setup_cancel_io(void)
Use_Duplicate_Handles?"":"Ex");
 }
 
-static inline BOOL cancel_io(int _index)
+static inline BOOL cancel_io(struct winfd * wfd)
 {
-   if ((_index < 0) || (_index >= MAX_FDS)) {
-   return FALSE;
-   }
-
-   if ( (poll_fd[_index].fd < 0) || (poll_fd[_index].handle == 
INVALID_HANDLE_VALUE)
- || (poll_fd[_index].handle == 0) || (poll_fd[_index].overlapped == 
NULL) ) {
+   if ( (wfd->handle == INVALID_HANDLE_VALUE)
+ || (wfd->handle == 0) || (wfd->overlapped == NULL) ) {
return TRUE;
}
-   if (poll_fd[_index].itransfer && poll_fd[_index].cancel_fn) {
+   if (wfd->itransfer && wfd->cancel_fn) {
// Cancel outstanding transfer via the specific callback
-   (*poll_fd[_index].cancel_fn)(poll_fd[_index].itransfer);
+   (*wfd->cancel_fn)(wfd->itransfer);
return TRUE;
}
if (pCancelIoEx != NULL) {
-   return (*pCancelIoEx)(poll_fd[_index].handle, 
poll_fd[_index].overlapped);
+   return (*pCancelIoEx)(wfd->handle, wfd->overlapped);
}
-   if (_poll_fd[_index].thread_id == GetCurrentThreadId()) {
-   return CancelIo(poll_fd[_index].handle);
+   if (wfd->thread_id == GetCurrentThreadId()) {
+   return CancelIo(wfd->handle);
}
usbi_warn(NULL, "Unable to cancel I/O that was started from another 
thread");
return FALSE;
@@ -132,18 +121,15 @@ static __inline void setup_cancel_io()
// No setup needed on WinCE
 }
 
-static __inline BOOL cancel_io(int _index)
+static __inline BOOL cancel_io(struct winfd * wfd)
 {
-   if ((_index < 0) || (_index >= MAX_FDS)) {
-   return FALSE;
-   }
-   if ( (poll_fd[_index].fd < 0) || (poll_fd[_index].handle == 
INVALID_HANDLE_VALUE)
- || (poll_fd[_index].handle == 0) || (poll_fd[_index].overlapped == 
NULL) ) {
+   if ( (wfd->handle == INVALID_HANDLE_VALUE)
+ || (wfd->handle == 0) || (wfd->overlapped == NULL) ) {
return TRUE;
}
-   if (poll_fd[_index].itransfer && poll_fd[_index].cancel_fn) {
+   if (wfd->itransfer && wfd->cancel_fn) {
// Cancel outstanding transfer via the specific callback
-   (*poll_fd[_index].cancel_fn)(poll_fd[_index].itransfer);
+   (*wfd->cancel_fn)(wfd->itransfer);
}
return TRUE;
 }
@@ -152,45 +138,16 @@ static __inline BOOL cancel_io(int _index)
 // Init
 void init_polling(void)
 {
-   int i;
-
while (InterlockedExchange((LONG *)&compat_spinlock, 1) == 1) {
SleepEx(0, TRUE);
}
if (!is_polling_set) {
setup_cancel_io();
-   for (i=0; iInternal = STATUS_PENDING;
-   overlapped->InternalHigh = 0;
-
-   for (i=0; i= 0) {
-   LeaveCriticalSection(&_poll_fd[i].mutex);
-   continue;
-   }
-
-   // Use index as the unique fd number
-   poll_fd[i].fd = i;
-   // Read end of the "pipe"
-   filedes[0] = poll_fd[i].fd;
-   // We can use the same handle for both ends
-   filedes[1] = filedes[0];
-
-   poll_fd[i].handle = DUMMY_HANDLE;
-   poll_fd[i].overlapped = overlapped;
-   // There's no polling on the write end, so we just use 
READ for our needs
-   poll_fd[i].rw = RW_READ;
-   _poll_fd[i].original_handle = INVALID_HANDLE_VALUE;
-   LeaveCriticalSection(&_poll_fd[i].mutex);
-   return 0;
-   }
+   if (!Duplica

[Libusbx-devel] [PATCH 11/11] Windows: Set limit of pollfds for Windows and Windows CE.

[Toby Gray]

Both the Windows and Windows CE backends use MultipleWaitForObjects
in their usbi_poll implementations.

This function has a limited number of objects it can wait on. This commit
configures libusb to the correct limit for maximum pollfds.
---
 configure.ac  |1 +
 msvc/config.h |3 +++
 2 files changed, 4 insertions(+)

diff --git a/configure.ac b/configure.ac
index 6d6b35d..153ff86 100644
--- a/configure.ac
+++ b/configure.ac
@@ -130,6 +130,7 @@ windows)
LIBS=""
LTLDFLAGS="${LTLDFLAGS} -avoid-version -Wl,--add-stdcall-alias"
AC_DEFINE([POLL_NFDS_TYPE],[unsigned int],[type of second poll() 
argument])
+   
AC_DEFINE([MAXIMUM_POLL_FDS],[MAXIMUM_WAIT_OBJECTS],[WaitForMultipleObjects 
supports a limited number of objects])
;;
 esac
 
diff --git a/msvc/config.h b/msvc/config.h
index bb542c5..1eaf20c 100644
--- a/msvc/config.h
+++ b/msvc/config.h
@@ -28,6 +28,9 @@
 /* type of second poll() argument */
 #define POLL_NFDS_TYPE unsigned int
 
+/* WaitForMultipleObjects supports a limited number of objects */
+#define MAXIMUM_POLL_FDS MAXIMUM_WAIT_OBJECTS
+
 /* Windows/WinCE backend */
 #if defined(_WIN32_WCE)
 #define OS_WINCE 1
-- 
1.7.9.5


--
This SF.net email is sponsored by Windows:

Build for Windows Store.

http://p.sf.net/sfu/windows-dev2dev
___
libusbx-devel mailing list
libusbx-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libusbx-devel

[Libusbx-devel] [PATCH 01/11] Tests: Adding ability for tests to use setjmp and longjmp for status reporting

[Toby Gray]

This improves the testlib used in libusbx to allow tests to either
return the test status or to call a function which never returns.

This leads to simpler test code as nested functions do not have to
return test status up the calling chain.
---
 tests/libusbx_testlib.h |   54 
 tests/stress.c  |8 ++--
 tests/testlib.c |  105 +--
 3 files changed, 160 insertions(+), 7 deletions(-)

diff --git a/tests/libusbx_testlib.h b/tests/libusbx_testlib.h
index 06dbc8e..8b109a2 100644
--- a/tests/libusbx_testlib.h
+++ b/tests/libusbx_testlib.h
@@ -21,6 +21,8 @@
 #define LIBUSBX_TESTLIB_H
 
 #include 
+#include 
+#include "libusb.h"
 
 #if !defined(bool)
 #define bool int
@@ -32,6 +34,15 @@
 #define false (!true)
 #endif
 
+/**
+ * Helper macro used to concatinate two values
+ */
+#define LIBUSBX_CONCAT_(a, b) a ## b
+/**
+ * Used to concatinate two strings in the preprocessor
+ */
+#define LIBUSBX_CONCAT(a, b) LIBUSBX_CONCAT_(a, b)
+
 /** Values returned from a test function to indicate test result */
 typedef enum {
/** Indicates that the test ran successfully. */
@@ -57,6 +68,10 @@ typedef struct {
int old_stderr;
FILE* output_file;
int null_fd;
+   char * test_device;
+   int test_device_bus;
+   int test_device_address;
+   jmp_buf test_return_buf;
 } libusbx_testlib_ctx;
 
 /**
@@ -90,6 +105,12 @@ typedef struct {
 #define LIBUSBX_NULL_TEST {NULL, NULL}
 
 /**
+ * Value to use in a test array for a test with the provided name
+ * defined in a function named with a test_ prefix.
+ */
+#define LIBUSBX_NAMED_TEST(name) {#name, &LIBUSBX_CONCAT(test_, name)}
+
+/**
  * Runs the tests provided.
  *
  * Before running any tests argc and argv will be processed
@@ -104,4 +125,37 @@ int libusbx_testlib_run_tests(int argc,
   char ** argv,
   const libusbx_testlib_test * tests);
 
+/**
+ * Exits the current test with the provided test status.
+ *
+ * This function makes use of longjmp so never returns.
+ */
+void libusbx_testlib_finish_current_test(libusbx_testlib_ctx * tctx,
+ libusbx_testlib_result result);
+
+/**
+ * Skips the current test if no test device has been provided.
+ *
+ * This function makes use of libusbx_testlib_finish_current_test
+ * so may not return.
+ */
+void libusbx_testlib_skip_if_no_device(libusbx_testlib_ctx * tctx);
+
+/**
+ * Initialise and return a libusb context or immediately fail the test.
+ *
+ * This function makes use of libusbx_testlib_finish_current_test
+ * so may not return.
+ */
+libusb_context * libusbx_testlib_init_ctx_or_fail(libusbx_testlib_ctx * tctx);
+
+/**
+ * Open and return a device handle for the test device or fail the test.
+ *
+ * This function makes use of libusbx_testlib_finish_current_test
+ * so may not return.
+ */
+libusb_device_handle * libusbx_testlib_open_test_device(
+   libusbx_testlib_ctx * tctx, libusb_context * ctx);
+
 #endif //LIBUSBX_TESTLIB_H
diff --git a/tests/stress.c b/tests/stress.c
index e53f415..f64f4b7 100644
--- a/tests/stress.c
+++ b/tests/stress.c
@@ -146,10 +146,10 @@ static libusbx_testlib_result 
test_default_context_change(libusbx_testlib_ctx *
 
 /* Fill in the list of tests. */
 static const libusbx_testlib_test tests[] = {
-   {"init_and_exit", &test_init_and_exit},
-   {"get_device_list", &test_get_device_list},
-   {"many_device_lists", &test_many_device_lists},
-   {"default_context_change", &test_default_context_change},
+   LIBUSBX_NAMED_TEST(init_and_exit),
+   LIBUSBX_NAMED_TEST(get_device_list),
+   LIBUSBX_NAMED_TEST(many_device_lists),
+   LIBUSBX_NAMED_TEST(default_context_change),
LIBUSBX_NULL_TEST
 };
 
diff --git a/tests/testlib.c b/tests/testlib.c
index e69aa39..414695c 100644
--- a/tests/testlib.c
+++ b/tests/testlib.c
@@ -20,6 +20,7 @@
 #include "libusbx_testlib.h"
 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -51,6 +52,12 @@
 #define IGNORE_RETVAL(expr) do { (void)(expr); } while(0)
 
 /**
+ * As setjmp and longjmp require non-zero values, this
+ * value is added to results before calling longjmp.
+ */
+#define TEST_JMP_RESULT_OFFSET 0x100
+
+/**
  * Converts a test result code into a human readable string.
  */
 static const char* test_result_to_str(libusbx_testlib_result result)
@@ -71,9 +78,10 @@ static const char* test_result_to_str(libusbx_testlib_result 
result)
 
 static void print_usage(int argc, char ** argv)
 {
-   printf("Usage: %s [-l] [-v] [ ...]\n",
+   printf("Usage: %s [-l] [-s :] [-v] [ ...]\n",
argc > 0 ? argv[0] : "test_*");
printf("   -l   List available tests\n");
+   printf("   -s   Use the device at the provided bus and address\n");
printf("   -v   Don't redirect STDERR/STDOUT during tests\n");
 }
 
@@ -183,6 +191,9 @@ int libusbx_testlib_run_tests(int argc,
c

[Libusbx-devel] [PATCH 09/11] Core: Allow backends to reserve pollfds.

[Toby Gray]

This adds in new internal APIs to allow backends to reserve the pollfds
which will be added to the pollfds linked list.

This also adds a new function for adding pre-reserved pollfds to the
pollfds linked list. This function can never fail, unlike the standard
usbi_add_pollfds().
---
 libusb/io.c  |   50 +-
 libusb/libusbi.h |5 +
 2 files changed, 50 insertions(+), 5 deletions(-)

diff --git a/libusb/io.c b/libusb/io.c
index d6eb3b5..f9056b2 100644
--- a/libusb/io.c
+++ b/libusb/io.c
@@ -1061,6 +1061,7 @@ int usbi_io_init(struct libusb_context *ctx)
list_init(&ctx->flying_transfers);
list_init(&ctx->pollfds);
ctx->pollfds_cnt = 0;
+   ctx->pollfds_reserved_cnt = 0;
 
/* FIXME should use an eventfd on kernels that support it */
r = usbi_pipe(ctx->ctrl_pipe);
@@ -2408,19 +2409,47 @@ void API_EXPORTED 
libusb_set_event_notifiers(libusb_context *ctx,
ctx->event_cb_user_data = user_data;
 }
 
-/* Add an event handle to the list of event handles to be monitored.
- * events should be specified as a bitmask of libusb_event_direction. */
-int usbi_add_pollfd(struct libusb_context *ctx, libusb_event_handle handle, 
short events)
+int usbi_reserve_pollfd(struct libusb_context *ctx, struct usbi_pollfd 
**ipollfd)
 {
-   struct usbi_pollfd *ipollfd = malloc(sizeof(*ipollfd));
-   if (!ipollfd)
+   int r = LIBUSB_SUCCESS;
+   usbi_mutex_lock(&ctx->pollfds_lock);
+#ifdef MAXIMUM_POLL_FDS
+   if (ctx->pollfds_reserved_cnt + ctx->pollfds_cnt >= MAXIMUM_POLL_FDS) {
+   usbi_err(ctx, "Number of pollfd would exceed OS limitation of 
%d",
+   MAXIMUM_POLL_FDS);
+   r = LIBUSB_ERROR_NOT_SUPPORTED;
+   } else
+#endif
+   ++ctx->pollfds_reserved_cnt;
+   usbi_mutex_unlock(&ctx->pollfds_lock);
+   if (r != LIBUSB_SUCCESS) {
+   *ipollfd = NULL;
+   return r;
+   }
+   *ipollfd = malloc(sizeof(**ipollfd));
+   if (!*ipollfd)
return LIBUSB_ERROR_NO_MEM;
+   return LIBUSB_SUCCESS;
+}
+
+void usbi_unreserve_pollfd(struct libusb_context *ctx, struct usbi_pollfd 
*ipollfd)
+{
+   usbi_mutex_lock(&ctx->pollfds_lock);
+   --ctx->pollfds_reserved_cnt;
+   usbi_mutex_unlock(&ctx->pollfds_lock);
+   free(ipollfd);
+}
+
 
+void usbi_add_reserved_pollfd(struct libusb_context *ctx,
+   libusb_event_handle handle, short events, struct usbi_pollfd *ipollfd)
+{
usbi_dbg("add handle %d events %d", handle, events);
ipollfd->pollfd.handle = handle;
ipollfd->pollfd.direction = poll_events_to_event_dirs(events);
usbi_mutex_lock(&ctx->pollfds_lock);
list_add_tail(&ipollfd->list, &ctx->pollfds);
+   --ctx->pollfds_reserved_cnt;
++ctx->pollfds_cnt;
usbi_mutex_unlock(&ctx->pollfds_lock);
 
@@ -2430,6 +2459,17 @@ int usbi_add_pollfd(struct libusb_context *ctx, 
libusb_event_handle handle, shor
 #endif
if (ctx->event_added_cb)
ctx->event_added_cb(handle, events, ctx->event_cb_user_data);
+}
+
+/* Add an event handle to the list of event handles to be monitored.
+ * events should be specified as a bitmask of libusb_event_direction. */
+int usbi_add_pollfd(struct libusb_context *ctx, libusb_event_handle handle, 
short events)
+{
+   struct usbi_pollfd *ipollfd;
+   int r = usbi_reserve_pollfd(ctx, &ipollfd);
+   if (r != LIBUSB_SUCCESS)
+   return r;
+   usbi_add_reserved_pollfd(ctx, handle, events, ipollfd);
return 0;
 }
 
diff --git a/libusb/libusbi.h b/libusb/libusbi.h
index 52d39ff..5380f4e 100644
--- a/libusb/libusbi.h
+++ b/libusb/libusbi.h
@@ -248,6 +248,7 @@ struct libusb_context {
/* list of poll fds */
struct list_head pollfds;
size_t pollfds_cnt;
+   size_t pollfds_reserved_cnt;
usbi_mutex_t pollfds_lock;
 
/* a counter that is set when we want to interrupt event handling, in 
order
@@ -471,6 +472,10 @@ struct usbi_pollfd {
 };
 
 int usbi_add_pollfd(struct libusb_context *ctx, libusb_event_handle handle, 
short events);
+void usbi_add_reserved_pollfd(struct libusb_context *ctx,
+   libusb_event_handle handle, short events, struct usbi_pollfd *ipollfd);
+int usbi_reserve_pollfd(struct libusb_context *ctx, struct usbi_pollfd 
**ipollfd);
+void usbi_unreserve_pollfd(struct libusb_context *ctx, struct usbi_pollfd 
*ipollfd);
 void usbi_remove_pollfd(struct libusb_context *ctx, libusb_event_handle 
handle);
 void usbi_fd_notification(struct libusb_context *ctx);
 
-- 
1.7.9.5


--
This SF.net email is sponsored by Windows:

Build for Windows Store.

http://p.sf.net/sfu/windows-dev2dev
___
libusbx-devel mailing list
libusbx-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libus

Re: [Libusbx-devel] Improved logging v2

[Pete Batard]

On 2013.06.27 16:15, Hans de Goede wrote:
> Thanks, I've pushed the 1st patch,

Dammit Hans, can we please avoid [r|p]ushing _new_ core features when 
we're smack down the middle of an RC? Or at least, if you deem we really 
must have them, can you wait at least 24 hours before committing, so 
that they have a chance to be reviewed and/or tested on platforms that 
are known to be capricious (looking at you cygwin!)?

While I appreciate Toby's effort, and I don't have any objections to 
_proposals_ being submitted whenever, even during an RC, I don't think 
_integrating_ this patch is something that should have been taking place 
during the RC.
In most other projects I know, RC is really a freeze-off time during 
which only critical or trivial patches get integrated. To me, this patch 
doesn't qualify as one of those.

Besides, you should be well aware that, with the number of platforms we 
have actually have to test for libusbx, there's too much of a risk that 
even a seemingly innocuous new patch will end up breaking one of the 
platforms that was greenlighted when we went to RC.

It's not a question of if, it's a question of when.

Or, in this case, it's not even a question of when. It's 
"Congratulations, this last commit just broke our 1.0.16 release on cygwin":
-
$ make -j2
make  all-recursive
make[1]: Entering directory `/d/libusbx'
Making all in libusb
make[2]: Entering directory `/d/libusbx/libusb'
   CC   libusb_1_0_la-core.lo
   CC   libusb_1_0_la-descriptor.lo
core.c: In function 'usbi_log_v':
core.c:2109:3: error: implicit declaration of function '_snprintf'
core.c:2124:2: error: implicit declaration of function '_vsnprintf'
Makefile:452: recipe for target `libusb_1_0_la-core.lo' failed
make[2]: *** [libusb_1_0_la-core.lo] Error 1
make[2]: *** Waiting for unfinished jobs
make[2]: Leaving directory `/d/libusbx/libusb'
Makefile:404: recipe for target `all-recursive' failed
make[1]: *** [all-recursive] Error 1
make[1]: Leaving directory `/d/libusbx'
Makefile:312: recipe for target `all' failed
make: *** [all] Error 2
-

At this stage, I'm almost tempted to just say "You break it, you fix 
it", but I'll see what I can do to sort things out. That is, unless Toby 
has more time than I do, and a cygwin platform he can test with. From 
what I can see, at least on my cygwin platform, __GCC__ is not defined 
(go figure - I don't remember changing anything from the defaults, so I 
expect other cygwins to be the same), hence the issue.
Also, we had the exact same #ifdef test that was added by this patch 10 
lines above (which I now have to wonder may also not produce the results 
we wanted for for cygwin), so we probably should have factorized the new 
code there.

Now, due to the number of new commits that have been applied since we 
declared RC, I would *strongly* suggest that we announce a new RC after 
this is fixed, and delay the release accordingly (one week after the 
announcement of RC2 is what I would see as a minimum). The rule should 
be that, if an RC breaks one platform, then a new RC should be declared 
after this is fixed. This is to avoid getting into a scenario where I 
decide that I don't have time to retest all the Windows platforms on a 
near daily basis, and we get an unpleasant surprise such as the one 
above post release.

> For the 2nd patch lets wait for Pete's comments.

That, and the cygwin fix, will depend on how far behind I get on what I 
was planning to get done for Rufus, which is what I really want to spend 
time on right now.

Regards,

/Pete

--
This SF.net email is sponsored by Windows:

Build for Windows Store.

http://p.sf.net/sfu/windows-dev2dev
___
libusbx-devel mailing list
libusbx-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libusbx-devel

Re: [Libusbx-devel] Improved logging v2

[Hans de Goede]

Hi,

On 06/27/2013 07:15 PM, Pete Batard wrote:

On 2013.06.27 16:15, Hans de Goede wrote:

Thanks, I've pushed the 1st patch,


Dammit Hans, can we please avoid [r|p]ushing _new_ core features when
we're smack down the middle of an RC? Or at least, if you deem we really
must have them, can you wait at least 24 hours before committing, so
that they have a chance to be reviewed and/or tested on platforms that
are known to be capricious (looking at you cygwin!)?

While I appreciate Toby's effort, and I don't have any objections to
_proposals_ being submitted whenever, even during an RC, I don't think
_integrating_ this patch is something that should have been taking place
during the RC.
In most other projects I know, RC is really a freeze-off time during
which only critical or trivial patches get integrated. To me, this patch
doesn't qualify as one of those.


You're right, and I'm sorry. I guess I'm to much of a rero person sometimes.

Although win32 is far from my specialty I think the attach patch should fix
it. Any chance you could give it a try?

Thanks,

Hans
>From 5ba318dad9eb16d7686abe5dd0ae4f44d1810b5a Mon Sep 17 00:00:00 2001
From: Toby Gray 
Date: Thu, 27 Jun 2013 16:47:46 +0100
Subject: [PATCH] Core: Fix warning in use of fprintf.

Some versions of GCC can check that format strings are being used
securely. This commit fixes the following warning:

  core.c:2023:2: warning: format not a string literal and no format arguments [-Wformat-security]

Signed-off-by: Hans de Goede 
---
 libusb/core.c | 2 +-
 libusb/version_nano.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/libusb/core.c b/libusb/core.c
index 53f716f..ec37ba1 100644
--- a/libusb/core.c
+++ b/libusb/core.c
@@ -2020,7 +2020,7 @@ int usbi_gettimeofday(struct timeval *tp, void *tzp)
 static void usbi_log_str(struct libusb_context *ctx, const char * str)
 {
 	UNUSED(ctx);
-	fprintf(stderr, str);
+	fprintf(stderr, "%s", str);
 }
 
 void usbi_log_v(struct libusb_context *ctx, enum libusb_log_level level,
diff --git a/libusb/version_nano.h b/libusb/version_nano.h
index 5ab4d98..41e8b15 100644
--- a/libusb/version_nano.h
+++ b/libusb/version_nano.h
@@ -1 +1 @@
-#define LIBUSB_NANO 10765
+#define LIBUSB_NANO 10766
-- 
1.8.3.1

--
This SF.net email is sponsored by Windows:

Build for Windows Store.

http://p.sf.net/sfu/windows-dev2dev___
libusbx-devel mailing list
libusbx-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libusbx-devel

Re: [Libusbx-devel] Improving linux hotplug disconnect

[Hans de Goede]

Hi,

On 06/27/2013 06:22 PM, Chris Dickens wrote:
> Hi,
>
>
> On Thu, Jun 27, 2013 at 3:31 AM, Hans de Goede  > wrote:
>
> Hi,
>
> +/* device will still be marked as attached if hotplug monitor thread
> +* hasn't processed remove event yet */
> +if (handle->dev->attached) {
>
>
> This check is racy, you need to take the hotplug lock before making this
> check, also linux_device_disconnected expects the hotplug lock to be
> taken when it is called, so simply but a check around the entire block.
>
>
> Ok, patch revised. With the current design this race condition is benign, but 
> better to be safe than sorry if something changes in the future.
>
> Ok, that makes sense, I can live with having a check in 2 places, please 
> respin with the suggested
> fixes and send with git send-email, or attach, so that your mail client 
> does not foobar the patch.
>
>
> Patch is attached.

Thanks, looks good.

But given that I've already broken stuff, thus delaying 1.0.16rc2 by merging 
Toby's
"Improved logging v2" patch, I think it is best to delay applying this till 
after we open
up development for 1.0.17.

Regards,

Hans

--
This SF.net email is sponsored by Windows:

Build for Windows Store.

http://p.sf.net/sfu/windows-dev2dev
___
libusbx-devel mailing list
libusbx-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libusbx-devel

Re: [Libusbx-devel] [RFC] Remove fake fds from Windows backends

[Hans de Goede]

Hi,

Cool stuff, thanks! I believe this is best reviewed by Pete though,
hopefully he can find some time for this in the not too distant
feature.

Enjoy your vacation / away from the office time :)

Regards,

Hans

On 06/27/2013 06:58 PM, Toby Gray wrote:
>
> I realise that 1.0.16 is currently trying to be released and that
> these patches won't get looked at until after that has happened. I'm
> away from the office for the next couple of weeks so thought it was
> worth sending this patch series for people to look at if they get
> time while I'm away.
>
> A while ago I mentioned that I thought that I was seeing performance
> issues due to all the fake FD logic in the Windows backends. This patch
> series makes these changes.
>
> This series is actually a group of related changes.
>
> Patches 1, 2 and 7 add some additional tests and testlib functionality.
>
> Patch 3 adds a new event API which provides FDs on POSIX systems but
> HANDLES on Windows systems.
>
> Patch 4 changes the core code to make use of the new FD/HANDLE type.
>
> Patch 5 and 6 change the Windows backends over to using the FD/HANDLE type.
>
> While developing these patches I noticed that the Windows backends have a bug
> where if you get more than MAXIMUM_WAIT_OBJECTS pollfds then any attempt at
> a synchronous transfer will hang. The problem is that the handling of
> events starts to fail when too many transfers have been submitted. This
> is impossible to recover from as event handling needs to occur to remove
> the pollfds for cancelled/completed transfers.
>
> Patches 8 and 9 modify the core code to allow pre-reserving of pollfds
> and limiting the total number of concurrent pollfds. Instead of failing
> in handling events with high numbers of pollfds this makes it possible
> to fail the submission of the transfer.
>
> Patch 10 makes use of pre-reserving of pollfds in the Windows backends.
>
> Finally patch 11 enables limiting the number of concurrent pollfds on
> Windows platforms.
>
> As for the original reason for this patch series, our actual
> performance uses cases have improved by about 5% on WinCE through
> these changes.
>
> Sadly the simple performance tests I created don't show any improvement
> as they are limited by IO scheduling, not CPU usage.
>
> Regards,
>
> Toby
>
> --
> This SF.net email is sponsored by Windows:
>
> Build for Windows Store.
>
> http://p.sf.net/sfu/windows-dev2dev
> ___
> libusbx-devel mailing list
> libusbx-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/libusbx-devel
>

--
This SF.net email is sponsored by Windows:

Build for Windows Store.

http://p.sf.net/sfu/windows-dev2dev
___
libusbx-devel mailing list
libusbx-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libusbx-devel

Re: [Libusbx-devel] Improved logging v2

[Tim Roberts]

Hans de Goede wrote:
> You're right, and I'm sorry. I guess I'm to much of a rero person sometimes.
>
> Although win32 is far from my specialty I think the attach patch should fix
> it. Any chance you could give it a try?

This is micro-optimization, of course, but it would be more efficient to
do this instead:
fputs(str, stderr);

-- 
Tim Roberts, t...@probo.com
Providenza & Boekelheide, Inc.


--
This SF.net email is sponsored by Windows:

Build for Windows Store.

http://p.sf.net/sfu/windows-dev2dev
___
libusbx-devel mailing list
libusbx-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libusbx-devel

Re: [Libusbx-devel] Improved logging v2

[Pete Batard]

On 2013.06.27 19:33, Hans de Goede wrote:
> I think the attach patch should fix it. Any chance you could give it a try?

Tested. Same failure. This isn't the issue (though yes, we want to fix 
that too).
The issue, as I pointed in my last e-mail, is 
https://github.com/libusbx/libusbx/blob/master/libusb/libusbi.h#L455
__GCC__ is not defined in my cywgin environment (and presumably others), 
and neither are _snprintf or _vsnprintf, hence the issue.

We could have a quick and dirty fix that adds && !defined(__CYGWIN__) 
(or whatever the cygwin alias is), but then there's 
https://github.com/libusbx/libusbx/blob/master/libusb/libusbi.h#L443 
with the exact same condition, that needs to be investigated, because
1. We should able able to factorize that _snprintf or _vsnprintf stuff in
2. We're probably not doing what we actually want to do for 
usbi_gettimeofday on cygwin

This needs to be investigated carefully and tested. I'm still planning 
to do just that, but I don't know when.

Regards,

/Pete

--
This SF.net email is sponsored by Windows:

Build for Windows Store.

http://p.sf.net/sfu/windows-dev2dev
___
libusbx-devel mailing list
libusbx-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libusbx-devel