[PATCH] Look for DFU functional descriptor in the configuration descriptors

Stefan Schmidt stefan at datenfreihafen.org
Tue Nov 9 21:43:48 CET 2010


Hello.

On Mon, 2010-11-08 at 22:58, Tormod Volden wrote:
> On Mon, Nov 8, 2010 at 12:21 PM, Stefan Schmidt wrote:
> 
> Thanks for the fast response and a very helpful review. I will repost
> the patches (with cc) addressing all the issues you have raised. Until
> then, just a few quick comments.

Great. Thanks a lot.

> > I prefer a commit message starting with the area or source file it touches to
> > give the commit message a context.
> >
> > e.g. main: Look for DFU functional descriptor in the configuration descriptors
> 
> Yes, this is sometimes a difficult balance. I will try to follow your
> preference.

Thanks. I find it very useful when going through a log and being able to
directly see what area of code is touched by a commit.

> >> Hi, I am looking at the dfu-util code and trying to make it work with
> >> some devices I have (under Linux).
> >
> > What devices do you work with? I would like to compile a list with devices
> > dfu-util can work with or has known problems.
> 
> I am actually working on enhancing dfu-util to support some
> *cough*ST*cough* extensions. The protocol is different but there is so
> much common code it would be stupid not to keep it together. I have
> got it working, and the new code is mostly in a new file, but I would
> like to work on it more before I suggest it for merging.

If we are able to keep it in a separate file and have some quirks/hooks in the
generic implementation I'm happy to have support for such extensions.

> There are
> also parts of dfu-util (dfu suffix decoding, protocol detection etc) I
> would like to have improved before my stuff can be cleanly integrated.

Great, its all welcome.

> However, the present patches are free-standing from that work. From
> the current dfu-util code it seems that the functional descriptor
> retrieval issue is seen on other devices as well.

That one looks pretty good already. The timing patch worries me more. BUt thats
ina different mail.

> > I can life with this workaround. And if anybody is interested in porting it over
> > to libusb 1.0 let me know. :)
> 
> I would (over longer term) be interested in this. I think too many
> applications are based on the stagnated 0.1 version. The dfu-util code
> should be relatively easy to port.

Cool

> > +     int ret = 0;
> 
> > Do we really need the initial set to 0 here?
> 
> > +
> > +     conflen = (usb_device(udev))->config->wTotalLength;
> > +     cbuf = malloc(conflen);
> > +     conflen = usb_get_descriptor(udev, USB_DT_CONFIG, index, cbuf, conflen);
> > +     while (p + 1 < conflen) {
> 
> I was thinking of the possibility of usb_get_descriptor returning less
> than 2, in which case the "while" block is not entered where ret is
> set. Of course, in that case we probably have a bigger problem and
> should bail out. I admit this is "compact" coding.

Agreed. I would like it better to bail out. The variable init did already hide
some bugs from me. :)

regards
Stefan Schmidt



More information about the devel mailing list