In kfifo_in marco, one piece of code is arounded by if(0). This code in
introduced by Stefani Seibold <[email protected]> to suppress a compiler
warning. This warning is not there with the upgrade of gcc version.
This patch just remove this code.
---
include/linux/kfifo.h | 4 ----
1 files changed, 0 insertions(+), 4 deletions(-)
diff --git a/include/linux/kfifo.h b/include/linux/kfifo.h
index 10308c6..e7015bb 100644
--- a/include/linux/kfifo.h
+++ b/include/linux/kfifo.h
@@ -512,10 +512,6 @@ __kfifo_uint_must_check_helper( \
unsigned long __n = (n); \
const size_t __recsize = sizeof(*__tmp->rectype); \
struct __kfifo *__kfifo = &__tmp->kfifo; \
- if (0) { \
- typeof(__tmp->ptr_const) __dummy __attribute__ ((unused)); \
- __dummy = (typeof(__buf))NULL; \
- } \
(__recsize) ?\
__kfifo_in_r(__kfifo, __buf, __n, __recsize) : \
__kfifo_in(__kfifo, __buf, __n); \
--
1.7.5.4
On Thu, Oct 18, 2012 at 3:59 PM, Wei Yang <[email protected]> wrote:
> In kfifo_in marco, one piece of code is arounded by if(0). This code in
> introduced by Stefani Seibold <[email protected]> to suppress a compiler
> warning. This warning is not there with the upgrade of gcc version.
>
> This patch just remove this code.
Are you sure?
This code fragment looks like a compiler bomb to detect type mismatch to me...
--
Thanks,
//richard
Am Freitag, den 19.10.2012, 00:37 +0200 schrieb richard -rw- weinberger:
> On Thu, Oct 18, 2012 at 3:59 PM, Wei Yang <[email protected]> wrote:
> > In kfifo_in marco, one piece of code is arounded by if(0). This code in
> > introduced by Stefani Seibold <[email protected]> to suppress a compiler
> > warning. This warning is not there with the upgrade of gcc version.
> >
> > This patch just remove this code.
>
> Are you sure?
> This code fragment looks like a compiler bomb to detect type mismatch to me...
>
Yes, you are great! That was the reason why i made this peace of code.
So don't remove it!
On Fri, 19 Oct 2012, Stefani Seibold wrote:
> > > In kfifo_in marco, one piece of code is arounded by if(0). This code in
> > > introduced by Stefani Seibold <[email protected]> to suppress a compiler
> > > warning. This warning is not there with the upgrade of gcc version.
> > >
> > > This patch just remove this code.
> >
> > Are you sure?
> > This code fragment looks like a compiler bomb to detect type mismatch to me...
>
> Yes, you are great! That was the reason why i made this peace of code.
> So don't remove it!
As even you, the author of the code, got confused by it for a while,
adding an explanatory comment seems appropriate.
Thanks,
--
Jiri Kosina
SUSE Labs
On Fri, Oct 19, 2012 at 8:18 AM, Richard Yang
<[email protected]> wrote:
> On Fri, Oct 19, 2012 at 07:23:09AM +0200, Stefani Seibold wrote:
>>
>>Am Freitag, den 19.10.2012, 00:37 +0200 schrieb richard -rw- weinberger:
>>> On Thu, Oct 18, 2012 at 3:59 PM, Wei Yang <[email protected]> wrote:
>>> > In kfifo_in marco, one piece of code is arounded by if(0). This code in
>>> > introduced by Stefani Seibold <[email protected]> to suppress a compiler
>>> > warning. This warning is not there with the upgrade of gcc version.
>>> >
>>> > This patch just remove this code.
>>>
>>> Are you sure?
>>> This code fragment looks like a compiler bomb to detect type mismatch to me...
>>>
>>
>>Yes, you are great! That was the reason why i made this peace of code.
>>So don't remove it!
> Thanks for your confirmation.
>
> Hmm, would you give me a hint on how it check the type mismatch?
Here a small hint:
---cut---
struct x {
};
struct y {
};
#define check(a, b) \
({ \
if (0) { \
typeof(a) __dummy __attribute__ ((unused)); \
__dummy = (typeof(b))NULL; \
} \
})
int main(int argc, char **argv)
{
struct x _x;
struct y _y;
check(&_x, &_x); //okay
check(&_x, &_y); //will explode with "assignment from
incompatible pointer type"
return 0;
}
---cut---
--
Thanks,
//richard
On Fri, Oct 19, 2012 at 10:24 AM, Richard Yang
<[email protected]> wrote:
> got it, thanks~
BTW: The surrounding if (0) {} block could be removed in future.
I'm sure Stefani added it to suppress a compiler warning.
But the typeof() magic has to stay...
--
Thanks,
//richard
On Fri, 19 Oct 2012 09:34:43 +0200 (CEST)
Jiri Kosina <[email protected]> wrote:
> On Fri, 19 Oct 2012, Stefani Seibold wrote:
>
> > > > In kfifo_in marco, one piece of code is arounded by if(0). This code in
> > > > introduced by Stefani Seibold <[email protected]> to suppress a compiler
> > > > warning. This warning is not there with the upgrade of gcc version.
> > > >
> > > > This patch just remove this code.
> > >
> > > Are you sure?
> > > This code fragment looks like a compiler bomb to detect type mismatch to me...
> >
> > Yes, you are great! That was the reason why i made this peace of code.
> > So don't remove it!
>
> As even you, the author of the code, got confused by it for a while,
> adding an explanatory comment seems appropriate.
>
include/linux/typecheck.h?
On Tue, Oct 23, 2012 at 4:27 AM, Richard Yang
<[email protected]> wrote:
> On Mon, Oct 22, 2012 at 04:56:02PM -0700, Andrew Morton wrote:
>>On Fri, 19 Oct 2012 09:34:43 +0200 (CEST)
>>Jiri Kosina <[email protected]> wrote:
>>
>>> On Fri, 19 Oct 2012, Stefani Seibold wrote:
>>>
>>> > > > In kfifo_in marco, one piece of code is arounded by if(0). This code in
>>> > > > introduced by Stefani Seibold <[email protected]> to suppress a compiler
>>> > > > warning. This warning is not there with the upgrade of gcc version.
>>> > > >
>>> > > > This patch just remove this code.
>>> > >
>>> > > Are you sure?
>>> > > This code fragment looks like a compiler bomb to detect type mismatch to me...
>>> >
>>> > Yes, you are great! That was the reason why i made this peace of code.
>>> > So don't remove it!
>>>
>>> As even you, the author of the code, got confused by it for a while,
>>> adding an explanatory comment seems appropriate.
>>>
>>
>>include/linux/typecheck.h?
>
> Something like this?
>
> From f20c316bf87d5db0816a01705b7869526c0b2363 Mon Sep 17 00:00:00 2001
> From: Wei Yang <[email protected]>
> Date: Wed, 17 Oct 2012 16:45:49 +0800
> Subject: [PATCH] Replace the type check code with typecheck() in kfifo_in
>
> In kfifo_in marco, one piece of code which is arounded by if(0) will check the
> type of __tmp->ptr_const and __buf. If they are different type, there will
> output a warning during compiling. This piece of code is not self explaining
> and a little bit hard to understand.
>
> Based on Andrew Morton's suggestion, this patch replace this with typecheck()
> which will be easy to understand.
> ---
> include/linux/kfifo.h | 5 +----
> 1 files changed, 1 insertions(+), 4 deletions(-)
>
> diff --git a/include/linux/kfifo.h b/include/linux/kfifo.h
> index 10308c6..b48fe71 100644
> --- a/include/linux/kfifo.h
> +++ b/include/linux/kfifo.h
> @@ -512,10 +512,7 @@ __kfifo_uint_must_check_helper( \
> unsigned long __n = (n); \
> const size_t __recsize = sizeof(*__tmp->rectype); \
> struct __kfifo *__kfifo = &__tmp->kfifo; \
> - if (0) { \
> - typeof(__tmp->ptr_const) __dummy __attribute__ ((unused)); \
> - __dummy = (typeof(__buf))NULL; \
> - } \
> + typecheck(typeof(__tmp->ptr_const), __buf);\
> (__recsize) ?\
> __kfifo_in_r(__kfifo, __buf, __n, __recsize) : \
> __kfifo_in(__kfifo, __buf, __n); \
What about all the other open coded type checks in that file?
--
Thanks,
//richard
On Tue, 23 Oct 2012 21:25:46 +0200
richard -rw- weinberger <[email protected]> wrote:
> > Subject: [PATCH] Replace the type check code with typecheck() in kfifo_in
> >
> > In kfifo_in marco, one piece of code which is arounded by if(0) will check the
> > type of __tmp->ptr_const and __buf. If they are different type, there will
> > output a warning during compiling. This piece of code is not self explaining
> > and a little bit hard to understand.
> >
> > Based on Andrew Morton's suggestion, this patch replace this with typecheck()
> > which will be easy to understand.
> > ---
> > include/linux/kfifo.h | 5 +----
> > 1 files changed, 1 insertions(+), 4 deletions(-)
> >
> > diff --git a/include/linux/kfifo.h b/include/linux/kfifo.h
> > index 10308c6..b48fe71 100644
> > --- a/include/linux/kfifo.h
> > +++ b/include/linux/kfifo.h
> > @@ -512,10 +512,7 @@ __kfifo_uint_must_check_helper( \
> > unsigned long __n = (n); \
> > const size_t __recsize = sizeof(*__tmp->rectype); \
> > struct __kfifo *__kfifo = &__tmp->kfifo; \
> > - if (0) { \
> > - typeof(__tmp->ptr_const) __dummy __attribute__ ((unused)); \
> > - __dummy = (typeof(__buf))NULL; \
> > - } \
> > + typecheck(typeof(__tmp->ptr_const), __buf);\
> > (__recsize) ?\
> > __kfifo_in_r(__kfifo, __buf, __n, __recsize) : \
> > __kfifo_in(__kfifo, __buf, __n); \
>
> What about all the other open coded type checks in that file?
Yes, there are a lot. Searching for "if (0)" finds them.