| Message ID | 20150406180541.7031.97131.stgit@nfdev2.cica.es |
|---|---|
| State | Accepted |
| Delegated to: | Pablo Neira |
| Headers | show |
On 04/06/2015 11:05 AM, Arturo Borrero Gonzalez wrote: > linux_list.h:381:59: warning: right-hand operand of comma expression has no effect [-Wunused-value] > for (pos = list_entry((head)->next, typeof(*pos), member), \ > ^ > libiptc.c:552:2: note: in expansion of macro 'list_for_each_entry' > list_for_each_entry(c, &h->chains, list) { > ^ > > [ Patch copied from one similar of Patrick McHardy on libnftnl ] > > Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com> > --- > libiptc/linux_list.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libiptc/linux_list.h b/libiptc/linux_list.h > index abdcf88..559e33c 100644 > --- a/libiptc/linux_list.h > +++ b/libiptc/linux_list.h > @@ -27,7 +27,7 @@ > 1; \ > }) > > -#define prefetch(x) 1 > +#define prefetch(x) ((void)0) > > /* empty define to make this work in userspace -HW */ > #define smp_wmb() > Why not just use "do {} while (0)"? I know that is what is used in the kernel for functions that don't do anything. - Alex -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tuesday 2015-04-07 02:38, Alexander Duyck wrote: >On 04/06/2015 11:05 AM, Arturo Borrero Gonzalez wrote: >> linux_list.h:381:59: warning: right-hand operand of comma expression has no effect [-Wunused-value] >> for (pos = list_entry((head)->next, typeof(*pos), member), \ >> ^ >> libiptc.c:552:2: note: in expansion of macro 'list_for_each_entry' >> list_for_each_entry(c, &h->chains, list) { >> ^ >> >> [ Patch copied from one similar of Patrick McHardy on libnftnl ] >> >> Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com> >> --- >> libiptc/linux_list.h | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/libiptc/linux_list.h b/libiptc/linux_list.h >> index abdcf88..559e33c 100644 >> --- a/libiptc/linux_list.h >> +++ b/libiptc/linux_list.h >> @@ -27,7 +27,7 @@ >> 1; \ >> }) >> >> -#define prefetch(x) 1 >> +#define prefetch(x) ((void)0) >> >> /* empty define to make this work in userspace -HW */ >> #define smp_wmb() >> > >Why not just use "do {} while (0)"? I know that is what is used in the >kernel for functions that don't do anything. I may be getting the terms wrong, but: do{}while(0) is not an expression, it is a (block) control statement. In particular, do{}while(0) won't evaluate to an rvalue. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 04/07/2015 01:37 AM, Jan Engelhardt wrote: > On Tuesday 2015-04-07 02:38, Alexander Duyck wrote: > >> On 04/06/2015 11:05 AM, Arturo Borrero Gonzalez wrote: >>> linux_list.h:381:59: warning: right-hand operand of comma expression has no effect [-Wunused-value] >>> for (pos = list_entry((head)->next, typeof(*pos), member), \ >>> ^ >>> libiptc.c:552:2: note: in expansion of macro 'list_for_each_entry' >>> list_for_each_entry(c, &h->chains, list) { >>> ^ >>> >>> [ Patch copied from one similar of Patrick McHardy on libnftnl ] >>> >>> Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com> >>> --- >>> libiptc/linux_list.h | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/libiptc/linux_list.h b/libiptc/linux_list.h >>> index abdcf88..559e33c 100644 >>> --- a/libiptc/linux_list.h >>> +++ b/libiptc/linux_list.h >>> @@ -27,7 +27,7 @@ >>> 1; \ >>> }) >>> >>> -#define prefetch(x) 1 >>> +#define prefetch(x) ((void)0) >>> >>> /* empty define to make this work in userspace -HW */ >>> #define smp_wmb() >>> >> Why not just use "do {} while (0)"? I know that is what is used in the >> kernel for functions that don't do anything. > I may be getting the terms wrong, but: > do{}while(0) is not an expression, it is a (block) control statement. > In particular, do{}while(0) won't evaluate to an rvalue. Right. That is the point in this case. I am assuming what Arturo is trying to accomplish since you shouldn't be able to evaluate ((void)0) as an rvalue either. The prefetch(x) is an empty statement which is being cast as a void to avoid a compiler warning, so it falls into the first case as defined in: http://kernelnewbies.org/FAQ/DoWhile0 - Alex -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tuesday 2015-04-07 19:45, Alexander Duyck wrote: >>>> -#define prefetch(x) 1 >>>> +#define prefetch(x) ((void)0) >>>> >>> Why not just use "do {} while (0)"? I know that is what is used in the >>> kernel for functions that don't do anything. >> I may be getting the terms wrong, but: >> do{}while(0) is not an expression, it is a (block) control statement. >> In particular, do{}while(0) won't evaluate to an rvalue. > > Right. That is the point in this case. I am assuming what Arturo is trying to > accomplish since you shouldn't be able to evaluate ((void)0) as an rvalue > either. The difference is that int i; i = 1, (void)0; compiles, while i = 1, do{}while(0); will not. Even less so with int i = 1, (void)0 vs int i = 1, do{}while(0); [that looks so obscure that even I need to go figure out why that is even permitted at this point]. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 04/07/2015 11:17 AM, Jan Engelhardt wrote: > > On Tuesday 2015-04-07 19:45, Alexander Duyck wrote: >>>>> -#define prefetch(x) 1 >>>>> +#define prefetch(x) ((void)0) >>>>> >>>> Why not just use "do {} while (0)"? I know that is what is used in the >>>> kernel for functions that don't do anything. >>> I may be getting the terms wrong, but: >>> do{}while(0) is not an expression, it is a (block) control statement. >>> In particular, do{}while(0) won't evaluate to an rvalue. >> >> Right. That is the point in this case. I am assuming what Arturo is trying to >> accomplish since you shouldn't be able to evaluate ((void)0) as an rvalue >> either. > > The difference is that > > int i; > i = 1, (void)0; > > compiles, while > > i = 1, do{}while(0); > Okay, that explains it. It is being used inside the init, condition, and post process sections of a for loop. Thanks for explaining it. - Alex -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Apr 06, 2015 at 08:05:41PM +0200, Arturo Borrero Gonzalez wrote: > linux_list.h:381:59: warning: right-hand operand of comma expression has no effect [-Wunused-value] > for (pos = list_entry((head)->next, typeof(*pos), member), \ > ^ > libiptc.c:552:2: note: in expansion of macro 'list_for_each_entry' > list_for_each_entry(c, &h->chains, list) { > ^ > > [ Patch copied from one similar of Patrick McHardy on libnftnl ] Also applied, thanks. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 06 Apr 2015 20:05:41 +0200 Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com> wrote: > linux_list.h:381:59: warning: right-hand operand of comma expression has no effect [-Wunused-value] > for (pos = list_entry((head)->next, typeof(*pos), member), \ > ^ > libiptc.c:552:2: note: in expansion of macro 'list_for_each_entry' > list_for_each_entry(c, &h->chains, list) { > ^ > > [ Patch copied from one similar of Patrick McHardy on libnftnl ] > > Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com> > --- > libiptc/linux_list.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libiptc/linux_list.h b/libiptc/linux_list.h > index abdcf88..559e33c 100644 > --- a/libiptc/linux_list.h > +++ b/libiptc/linux_list.h > @@ -27,7 +27,7 @@ > 1; \ > }) > > -#define prefetch(x) 1 > +#define prefetch(x) ((void)0) > > /* empty define to make this work in userspace -HW */ > #define smp_wmb() > > -- > To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html The kernel dropped the prefetch() in the list macros a couple of years ago. The prefetch() had no performance gain and hurt in several cases. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Apr 08, 2015 at 05:18:25PM -0700, Stephen Hemminger wrote: > On Mon, 06 Apr 2015 20:05:41 +0200 > Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com> wrote: > > > linux_list.h:381:59: warning: right-hand operand of comma expression has no effect [-Wunused-value] > > for (pos = list_entry((head)->next, typeof(*pos), member), \ > > ^ > > libiptc.c:552:2: note: in expansion of macro 'list_for_each_entry' > > list_for_each_entry(c, &h->chains, list) { > > ^ > > > > [ Patch copied from one similar of Patrick McHardy on libnftnl ] > > > > Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com> > > --- > > libiptc/linux_list.h | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/libiptc/linux_list.h b/libiptc/linux_list.h > > index abdcf88..559e33c 100644 > > --- a/libiptc/linux_list.h > > +++ b/libiptc/linux_list.h > > @@ -27,7 +27,7 @@ > > 1; \ > > }) > > > > -#define prefetch(x) 1 > > +#define prefetch(x) ((void)0) > > > > /* empty define to make this work in userspace -HW */ > > #define smp_wmb() > > > > -- > > To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in > > the body of a message to majordomo@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > The kernel dropped the prefetch() in the list macros a couple > of years ago. The prefetch() had no performance gain and hurt > in several cases. We have an old copy of linux's list.h in our userspace trees. We can probably refresh those to get them in sync with current at some point. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/libiptc/linux_list.h b/libiptc/linux_list.h index abdcf88..559e33c 100644 --- a/libiptc/linux_list.h +++ b/libiptc/linux_list.h @@ -27,7 +27,7 @@ 1; \ }) -#define prefetch(x) 1 +#define prefetch(x) ((void)0) /* empty define to make this work in userspace -HW */ #define smp_wmb()
linux_list.h:381:59: warning: right-hand operand of comma expression has no effect [-Wunused-value] for (pos = list_entry((head)->next, typeof(*pos), member), \ ^ libiptc.c:552:2: note: in expansion of macro 'list_for_each_entry' list_for_each_entry(c, &h->chains, list) { ^ [ Patch copied from one similar of Patrick McHardy on libnftnl ] Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com> --- libiptc/linux_list.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html