I'm not sure where you supplied the patch which adds timeouts.
Anyway, here's a patch which fixes it w/o adding hardcoded timeouts
(they're hardcoded inside libusb instead).
It also doesn't break the API.
diff --git a/src/ftdi.c b/src/ftdi.c
index 988a9f1..ed2c817 100644
--- a/src/ftdi.c
+++ b/src/ftdi.c
@@ -1884,22 +1884,16 @@ struct ftdi_transfer_control
*ftdi_read_data_submit(struct ftdi_context *ftdi, u
int ftdi_transfer_data_done(struct ftdi_transfer_control *tc)
{
int ret;
- struct timeval to = { 0, 0 };
while (!tc->completed)
{
- ret = libusb_handle_events_timeout_completed(tc->ftdi->usb_ctx,
- &to, &tc->completed);
+ ret = libusb_handle_events_completed(tc->ftdi->usb_ctx,
+ &tc->completed);
if (ret < 0)
{
if (ret == LIBUSB_ERROR_INTERRUPTED)
continue;
- libusb_cancel_transfer(tc->transfer);
- while (!tc->completed)
- if
(libusb_handle_events_timeout_completed(tc->ftdi->usb_ctx,
- &to, &tc->completed) < 0)
- break;
- libusb_free_transfer(tc->transfer);
- free (tc);
+
+ ftdi_transfer_data_cancel(tc, NULL);
return ret;
}
}
@@ -1931,18 +1925,23 @@ int ftdi_transfer_data_done(struct
ftdi_transfer_control *tc)
void ftdi_transfer_data_cancel(struct ftdi_transfer_control *tc,
struct timeval * to)
{
- struct timeval tv = { 0, 0 };
-
+ int ret;
if (!tc->completed && tc->transfer != NULL)
{
- if (to == NULL)
- to = &tv;
-
libusb_cancel_transfer(tc->transfer);
while (!tc->completed)
{
- if (libusb_handle_events_timeout_completed(tc->ftdi->usb_ctx,
to, &tc->completed) < 0)
+ if (to != NULL) {
+ ret =
libusb_handle_events_timeout_completed(tc->ftdi->usb_ctx,
+ to, &tc->completed) < 0;
+ } else {
+ ret = libusb_handle_events_completed(tc->ftdi->usb_ctx,
+ &tc->completed) < 0;
+ }
+
+ if (ret < 0) {
break;
+ }
}
}
On Wed, Oct 2, 2019 at 2:28 PM Thomas Jarosch <[email protected]>
wrote:
> Hi Omri,
>
> You wrote on Thu, Aug 22, 2019 at 11:22:35AM +0300:
> > It seems that both ftdi_transfer_data_done and ftdi_transfer_data_cancel
> > call libusb_handle_events_timeout_completed with zero timeval, with the
> > intention of indefinitely blocking (it even states so in
> > ftdi_transfer_data_cancel's documentation).
> > However, in practice, libusb_handle_events_timeout_completed with zero
> > timeval just acts as a non-blocking function, which means both of these
> > functions actually busy-wait instead of polling indefinitely.
> > Am I missing something?
> > Shall I propose a patch?
>
> ftdi_transfer_data_cancel() allows the user to pass an optional timeout.
>
> ftdi_transfer_data_done() lacks this API and we can't
> easily extend it without breaking existing users.
>
> I've checked the libusb_handle_events_timeout_completed() API
> documentation,
> theoretically it should not make much of a difference in user visible
> behavior if we specify a timeout of one second or so.
>
> CC:ing Eugene as he might know this code more than me.
> (=read: Supplied the patch that added the timeouts)
>
> [second email]
> > Do you think it's worth fixing for the next release?
> > I can write up a patch and manually test it.
>
> if there's really no user visible change, I think it's worth it
> as it will make the code more efficient and reduce power consumption.
>
> Cheers,
> Thomas
>
> --
> libftdi - see http://www.intra2net.com/en/developer/libftdi for details.
> To unsubscribe send a mail to [email protected]
>
>
>
--
libftdi - see http://www.intra2net.com/en/developer/libftdi for details.
To unsubscribe send a mail to [email protected]
