On Tuesday 06 February 2007 1:08 pm, Inaky Perez-Gonzalez wrote:
> On Friday 02 February 2007 19:05, you wrote:
> > The code should have used sizes such as
> > USB_DT_CONFIG_SIZE instead of sizeof().
>
> I have a problem with that, and is maintenance. Is not that I don't like
> DT_CONFIG_SIZE at all (that I don't). I prefer to keep things centralized,
> that being the descriptor definition, mapping the specification definition.
Ditto. DT_* is IMO legacy of some old USB code. Best removed.
Hmm, maybe we could still blame it on Inaky ... he was doing Linux-USB
work way back then, maybe it was his header file we copied! ;)
> > It's elementary. The use of
> > __attribute__((packed)) adds nontrivial costs... on some architectures.
>
> It sure does, but does it offset the non-trivial costs we'd incur by
> extracting the data manually? Or maybe is the compiler the one that is
> broken by not being able to do automatically what we can do by hand?
>
> [btw, I truly have little idea about which are those specific costs,
> out of professional curiosity, got any pointers?]
>
> If we are talking about misaligned access exceptions...ain't that a
> compiler bug? compiler-generated asm that is accessing a packed
> struct should take that into account, and unfold it as neccessary to
> avoid misalignments.
Exactly. Applied to a type, "packed" gets transferred to all its
members. Applied to such a member, "packed":
... specifies that a variable or structure field should
have the smallest possible alignment -- one byte for a
variable, one bit for a field, unless you specify a larger
value with the "aligned" attribute.
That's from gcc 3.3.4 info, and the text predates that version.
> Here is my point: it's way easier and more maintenable to do
>
> struct some_descriptor {
> __le16 foo;
> __u8 bar;
A better example would interchange the two sizes, of course... ;)
> } __attribute__((packed));
> ...
> struct some_descriptor descr;
> ...
> read_from_wire(&descr, sizeof(descr));
> do_something_with_values(&descr->foo, &descr->bar)
>
> than
>
> u8 buffer[SOME_DESCRIPTOR_DT_SIZE], foo;
> le16 bar;
> read_from_wire(buffer, SOME_DESCRIPTOR_DT_SIZE);
> bar = le16_to_cpu((__le16)buffer[1] << 8 | buffer[0]);
> foo = buffer[2];
> do_something_with_values(foo, bar)
>
> The compiler should be able to do that translation and deal with
> the alignment issues if any.
That is, the first case should be equivalent to the second, in
terms of the effect of executing it. In terms of readability,
maintainability ... the first is a clear win.
> Now, overhead we'll have in every arch when accessing bytes from the
> wire. Do we want to do that overhead by hand and make it a pain to
> maintain or do we want the compiler to do that job? Frankly, method
> #2 gives me the chills. It's ugly with a small descriptor, imagine
> with a huge one.
Yes. Yes. Yes.
I have said it thrice: what I tell you three times is true. ;)
And the compiler will know when those overhead costs can be eliminated;
if it's open-coded in the source, we'd be stuck with them always.
- Dave
-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier.
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
