Re: [PATCH] kfifo: remove unnecessary type check

From: Stefani Seibold
Date: Fri Oct 26 2012 - 02:51:53 EST


Am Freitag, den 26.10.2012, 14:11 +0800 schrieb Yuanhan Liu:
> On Fri, Oct 26, 2012 at 07:38:31AM +0200, Stefani Seibold wrote:
> > Am Freitag, den 26.10.2012, 09:46 +0800 schrieb Yuanhan Liu:
> > > From: Yuanhan Liu <yuanhan.liu@xxxxxxxxxxxxxxx>
> > >
> > > Firstly, this kind of type check doesn't work. It does something similay
> > > like following:
> > > void * __dummy = NULL;
> > > __buf = __dummy;
> > >
> > > __dummy is defined as void *. Thus it will not trigger warnings as
> > > expected.
> > >
> > > Second, we don't need that kind of check. Since the prototype
> > > of __kfifo_out is:
> > > unsigned int __kfifo_out(struct __kfifo *fifo, void *buf, unsigned int len)
> > >
> > > buf is defined as void *, so we don't need do the type check. Remove it.
> > >
> > > LINK: https://lkml.org/lkml/2012/10/25/386
> > > LINK: https://lkml.org/lkml/2012/10/25/584
> > >
> > > Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
> > > Cc: Wei Yang <weiyang@xxxxxxxxxxxxxxxxxx>
> > > Cc: Stefani Seibold <stefani@xxxxxxxxxxx>
> > > Cc: Fengguang Wu <fengguang.wu@xxxxxxxxx>
> > > Cc: Stephen Rothwell <sfr@xxxxxxxxxxxxxxxx>
> > > Signed-off-by: Yuanhan Liu <yuanhan.liu@xxxxxxxxxxxxxxx>
> > > ---

> >
> > Did you tried to compile the whole kernel including all the drivers with
> > your patch?
>
> Hi Stefani,
>
> I did a build test, it did't introduce any new compile errors and
> warnings. While, I haven't tried make allmodconfig then. Does this patch
> seems wrong to you?
>
> Thanks,
> Yuanhan Liu

Hi Liu,

no the patch seems not wrong to me. But as you see with the previous
patch it is not easy to predict the side effects.

An allmodconfig together with C=2 is necessary to check if there is no
side effects which current users of the kfifo API. That is exactly what
i did again and again as i developed the kfifo API.

Also you have to build the kfifo samples, since this example code use
all features of the kfifo API.

And again: The kfifo is designed to do the many things at compile time,
not at runtime. If you modify the code, you have to check the compiler
assembler output for no degradation, especially in kfifo_put, kfifo_get,
kfifo_in, kfifo_out, __kfifo_in and __kfifo_out. Prevent runtime checks
if you can do it at compile time. This is the basic reasons to do it in
macros.

Greetings,
Stefani


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