Re: PATCH [1/3] drivers/input/xpad.c: Improve Xbox 360 wireless support and add sysfs interface

[Mike Murphy]

On Mon, Mar 2, 2009 at 5:00 PM, Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> wrote:
>
> Yup, there's lots of crappy code in the tree, and it is regrettable
> that maintainers continue to go ahead and merge that crappy code.
>
> There's no easy fix for this - you need to be aware of what is right
> and what is wrong, but you cannot look at existing code to determine
> this :(

Part of the problem is that this is the first driver I've really
worked on to any extent... and all my formal kernel hacking training
was on FreeBSD, not Linux. Given the speed at which kernel development
moves, lack of good docs on how best to do development, etc., one
simply has to jump into the process and hope for the best. That's
awfully hard to do when kernel hacking isn't your full-time
occupation....

For example, my dissertation is on virtualization of grid computing
systems. I'm doing this driver development simply because I need the
driver for my own use... and perhaps one of our affiliated research
groups can use it. In my own research, I use Linux exclusively, but
all the work I do is at such a high level in userspace that I write
all my code in Python. I've taught C programming before and have done
a ton of low-level development in a C dialect for sensor networks
(nesC for TinyOS), so I'm comfortable with it. Even so, the kernel has
its own internal types (hence my unneeded typecasts), and it's not
always clear what type belongs where.

>
> If the code which you're modifying is known (by you) to be wrong then
> there are two schools of thought.  Some people do like to "match the
> existing code".  I disagree with that.  The code's wrong dammit - we
> might as well make the new code "right".  If that results in
> inconsistent-looking code, well, so be it.  We shouldn't have merged
> the wrong code in the first place.
>
>

Agreed on the code being correct, but there always arises that
question about what constitutes "correct" in any given context
(provably dumb things, like dividing by zero, excepted). For example,
I broke the xpad driver into two pieces to make it easier to
understand. When I go to compile it, the #include "xpad.h" in xpad.c
literally causes the preprocessor to dump the contents of xpad.h into
xpad.c, before handing the result off to the compiler for conversion
to assembly. So the system doesn't really care what goes in which
file... that distinction is left to the humans, who have to establish
some kind of convention. But where is that convention documented?

I could make the argument, for example, that the table of different
Xbox devices and their properties should really go in the header file,
even though it generates an array. That table constitutes metadata
that the driver uses to map device ids to products, so from a software
engineering perspective, it is more of a configuration than an
executable segment. On the other hand, the only piece that really
cares about that data is the driver itself... userspace really only
cares about what type of device is actually connected. So from that
perspective, it belongs in the C file. But then, whose userspace code
is really going to include xpad.h? Circles are fun....

And perhaps this is why we academics tend not to write operating
systems that find their way into widespread use... instead ruminating
on "nicer" designs (perhaps with a microkernel, *cough*) over more
practical solutions :).

Mike
-- 
Mike Murphy
Ph.D. Candidate and NSF Graduate Research Fellow
Clemson University School of Computing
120 McAdams Hall
Clemson, SC 29634-0974 USA
Tel: +1 864.656.2838   Fax: +1 864.656.0145
http://cirg.cs.clemson.edu/~mamurph
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/