On Fri, Jun 15, 2012 at 02:45:33PM -0700, Greg KH wrote:
> On Fri, Jun 15, 2012 at 08:40:26AM +0200, Julian Andres Klode wrote:
> > On Thu, Jun 14, 2012 at 03:09:38PM -0700, Greg KH wrote:
> > > On Thu, Jun 14, 2012 at 11:57:38PM +0200, Marc Dietrich wrote:
> > > > Add a helper macro to wrap nvec_{a}sync_writes and to get rid of
> > > > the various strings distributed all over the nvec code.
> > >
> > > Why can't these be inline functions instead? That will catch errors
> > > easier, and make it a bit more "obvious" as to what is going on (hint, I
> > > have no idea in reading these what they are doing...)
> >
> > They are not really obvious, but they kind of catch more errors and are shorter to
> > writer.
>
> Shorter to write where?
>
> And obvious is good, we want obvious in the kernel. Non-obvious is bad,
> bugs live there...
>
> > A nvec "call" consists of a type, a subtype and a payload, so
> > the calls expand like this:
> >
> > nvec_write_async(nvec, {NVEC_FOO, NVEC_FOO_BAR, ...}, length of ... + 2)
> > NVEC_CALL(nvec, FOO, BAR, ...)
> >
> > With an inline function, you would have to use arrays (as nvec_write_async()
> > does) or variable arguments lists which are not as optimizable, and you would
> > need to repeat NVEC all over the place. For example, with an inline function
> > instead of a macro:
> >
> > nvec_call(nvec, NVEC_FOO, NVEC_FOO_BAR, [size here?], ...)
> >
> > This manual size tracking also makes it less reliable.
>
> I'm sorry, but I still don't understand. Why would you ever want any
> NVEC_CALL macros either?
>
> What exactly is the driver doing here that is so "odd" from other data
> streams that need to be written to devices that it has to go through
> wierd gyrations like this?
The core point was probably that we currently have various sequences
like
char blah[] = {some hex value, other hex value, ...}
nvec_write_async(nvec, blah, sizeof(blah))
which is a bit long to write or distracting under some circumstances (and
not very obvious, as we did not use named constants for those hex values
most of the time). With a macro you could just write it:
NVEC_CALL(nvec, FOO, BAR)
And save a line. Would you prefer us to use:
{
char msg[] = {NVEC_FOO, NVEC_FOO_BAR};
nvec_write_async(nvec, msg, sizeof(msg));
}
instead? And yes, macro-less code is much more readable than the
macro code for non-insiders, which is probably helpful.
--
Julian Andres Klode - Debian Developer, Ubuntu Member
See http://wiki.debian.org/JulianAndresKlode and http://jak-linux.org/.
Attachment:
pgpCDMS6_ItKM.pgp
Description: PGP signature
_______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
