Quantcast

Suggested patch for adding ftdi_transfer_data_cancel

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Suggested patch for adding ftdi_transfer_data_cancel

Eugene Hutorny
Dear Colleagues,
 
I have met a problem with a submitted read request – it never ends if there is no input data.
When application calls ftdi_transfer_data_done it never returns.
libusb logs indicate that the transfer completes with status word only and then unconditionally resubmitted.
Exit from the app with no call to ftdi_transfer_data_done causes random crashes.
 
Suggested patch implements a cancelation routine and solves the problem I’ve described.
 
Best Regards
 
Eugene
 


libftdi - see http://www.intra2net.com/en/developer/libftdi for details.
To unsubscribe send a mail to [hidden email]



0001-Added-ftdi_transfer_data_cancel.patch (5K) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Suggested patch for adding ftdi_transfer_data_cancel

Thomas Jarosch
Hi Eugene,

Am 25.01.2016 um 08:35 schrieb Eugene Hutorny:

> I have met a problem with a submitted read request – it never ends if
> there is no input data.
> When application calls ftdi_transfer_data_done it never returns.
> libusb logs indicate that the transfer completes with status word only
> and then unconditionally resubmitted.
> Exit from the app with no call to ftdi_transfer_data_done causes random
> crashes.
>  
> Suggested patch implements a cancelation routine and solves the problem
> I’ve described.

thanks for the patch, applied! I've massaged the code indentation a bit,
since libftdi uses spaces for indentation instead of tabs.

While reviewing your changes, I wonder if
the existing libftdi code does the wrong thing:

+            if (ret < 0)
+                tc->completed = 1;



shouldn't that rather read:

if (ret <0)
    tc->completed = LIBUSB_TRANSFER_ERROR;

instead of hardcoding the enum value directly?

Cheers,
Thomas


--
libftdi - see http://www.intra2net.com/en/developer/libftdi for details.
To unsubscribe send a mail to [hidden email]  

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Suggested patch for adding ftdi_transfer_data_cancel

Eugene Hutorny
Hi Thomas,

In the existing code tc->completed is used as boolean.
Definitely it is not an error. Perhaps it would make sense to use
TRUE/FALSE,
but I am not sure if it worth refactoring.

Best Regards

Eugene


-----Original Message-----
From: Thomas Jarosch
Sent: Saturday, March 26, 2016 14:09
To: [hidden email] ; [hidden email]
Subject: Re: Suggested patch for adding ftdi_transfer_data_cancel

Hi Eugene,

Am 25.01.2016 um 08:35 schrieb Eugene Hutorny:

> I have met a problem with a submitted read request – it never ends if
> there is no input data.
> When application calls ftdi_transfer_data_done it never returns.
> libusb logs indicate that the transfer completes with status word only
> and then unconditionally resubmitted.
> Exit from the app with no call to ftdi_transfer_data_done causes random
> crashes.
>
> Suggested patch implements a cancelation routine and solves the problem
> I’ve described.

thanks for the patch, applied! I've massaged the code indentation a bit,
since libftdi uses spaces for indentation instead of tabs.

While reviewing your changes, I wonder if
the existing libftdi code does the wrong thing:

+            if (ret < 0)
+                tc->completed = 1;



shouldn't that rather read:

if (ret <0)
    tc->completed = LIBUSB_TRANSFER_ERROR;

instead of hardcoding the enum value directly?

Cheers,
Thomas


--
libftdi - see http://www.intra2net.com/en/developer/libftdi for details.
To unsubscribe send a mail to [hidden email]  

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Re: Suggested patch for adding ftdi_transfer_data_cancel

Thomas Jarosch
Hi Eugene,

On Monday, 28. March 2016 17:22:39 Eugene Hutorny wrote:
> In the existing code tc->completed is used as boolean.
> Definitely it is not an error. Perhaps it would make sense to use
> TRUE/FALSE,
> but I am not sure if it worth refactoring.

my point was that libftdi doesn't own the "tc" data structure. It's part
of libusb and we shouldn't write numerical values directly into it.

Luckily for us the values map to the desired behavior (f.e.
LIBUSB_TRANSFER_ERROR => 1) in libusb and are unlikely to change.

Cheers,
Thomas


--
libftdi - see http://www.intra2net.com/en/developer/libftdi for details.
To unsubscribe send a mail to [hidden email]  

Loading...