[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