Re: [PATCH 6/6] [C67x00] Merge c67x00-hub.c into c67x00-hcd.c

On Wed, 13 Jun 2007, Grant Likely wrote:

> On 6/12/07, Peter Korsgaard <[email protected]> wrote:
> > >>>>> "Grant" == Grant Likely <[email protected]> writes:
> >
> > Hi,
> >
> >  Grant> Rather than c67x00-hub.c being compiled seperately, the
> >  Grant> original code had c67x00-hub.c *included* by c67x00-hcd.c.
> >  Grant> This is a very bad idea.

What's so bad about it?  It's an elegant solution to the problem of 
breaking a very long driver up into smaller, more digestible pieces 
without polluting the kernel's namespace with lots of extra global 
symbols.

> >   Simplest solution is to merge the
> >  Grant> two files into one and be done with it.
> >
> > Yeah, it isn't exactly pretty, but it's what the other hcd drivers do,
> > E.G.:
> >
> > % grep -rs "include.*hub.c" *c
> > ehci-hcd.c:#include "ehci-hub.c"
> > ohci-hcd.c:#include "ohci-hub.c"
> > uhci-hcd.c:#include "uhci-hub.c"
> >
> > I don't quite know why they do it like that though ..
> 
> True, but that doesn't mean that it's a good idea to follow the lead.

Why not?

> There are lots of other examples of ugly code in the kernel that is
> tolerated just because nobody has cleaned it up yet, but is still
> unacceptable for new code.

What's so ugly about breaking a driver up into pieces?  Leaving it in 
one giant piece would be much more ugly IMO.

> We know it's an ugly thing to do, and the fix is simple and easy.

As the Firesign Theater once said, Everything you know is wrong!  :-)

Alan Stern


-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel