Re: p = kmalloc(sizeof(*p), )

[Randy.Dunlap]

On Tue, 20 Sep 2005, Pekka J Enberg wrote:

> On Tue, 20 Sep 2005, Al Viro wrote:
>
> > On Tue, Sep 20, 2005 at 11:53:42AM +0300, Pekka Enberg wrote:
> > > On 9/18/05, Robert Love <rml@xxxxxxxxxx> wrote:
> > > > 5.  Contrary to the above statement, such coding style does not help,
> > > >     but in fact hurts, readability.  How on Earth is sizeof(*p) more
> > > >     readable and information-rich than sizeof(struct foo)?  It looks
> > > >     like the remains of a 5,000 year old wolverine's spleen and
> > > >     conveys no information about the type of the object that is being
> > > >     created.
> > >
> > > Yes it does. The semantics are clearly "I want enough memory to hold
> > > the type this pointer points to." While sizeof(struct foo) might seem
> > > more readable, it is in fact not as you have no way of knowing whether
> > > the allocation is correct or not by looking at the line. So for
> > > spotting allocation errors with grep, the shorter form is better (and
> > > arguably less error-prone).
> >
> > Huh???   How do you use grep to find something of that sort?
>
> To find candidates, something like:
>
> grep "kmalloc(sizeof([^*]" -r drivers/ | grep -v "sizeof(struct"
>
> And then use my eyes to find real bugs.

ugh.

I guess I'm old-fashioned:  I think that the real answer is
to do the due diligence when making a patch.  Don't assume
that you/I/we can make a one-line change without checking
around/above/below it, where it's declared, allocated, etc.

I once worked with someone whose attitude was to "let the
compiler find all of the problems," so he threw quite the
garbage at it and iterated until the compiler no longer
complained.  I think that's the wrong way to do it, but
maybe it was his version of release early, release often.

-- 
~Randy
-
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/