[PATCH] xt_log.h: don't use xchg() for simple assignment

[PATCH] xt_log.h: don't use xchg() for simple assignment

[Jan Beulich]

At least on ia64 the (bogus) use of xchg() here results in the compiler
warning about an unused expression result. As only an assignment is
intended here, convert it to such.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

---
 include/net/netfilter/xt_log.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- 3.3-rc6/include/net/netfilter/xt_log.h
+++ 3.3-rc6-netfilter-xt_log-no-xchg/include/net/netfilter/xt_log.h
@@ -47,7 +47,7 @@ static void sb_close(struct sbuff *m)
 	if (likely(m != &emergency))
 		kfree(m);
 	else {
-		xchg(&emergency_ptr, m);
+		emergency_ptr = m;
 		local_bh_enable();
 	}
 }

Re: [PATCH] xt_log.h: don't use xchg() for simple assignment

[Pablo Neira Ayuso]

On Thu, Mar 08, 2012 at 09:45:44AM +0000, Jan Beulich wrote:
> At least on ia64 the (bogus) use of xchg() here results in the compiler
> warning about an unused expression result. As only an assignment is
> intended here, convert it to such.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> ---
>  include/net/netfilter/xt_log.h |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> --- 3.3-rc6/include/net/netfilter/xt_log.h
> +++ 3.3-rc6-netfilter-xt_log-no-xchg/include/net/netfilter/xt_log.h
> @@ -47,7 +47,7 @@ static void sb_close(struct sbuff *m)
>  	if (likely(m != &emergency))
>  		kfree(m);
>  	else {
> -		xchg(&emergency_ptr, m);
> +		emergency_ptr = m;

That xchg is there for some reason. I think it's better if you just
cast it to void.

Re: [PATCH] xt_log.h: don't use xchg() for simple assignment

[Jan Beulich]

>>> On 23.03.12 at 11:07, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Thu, Mar 08, 2012 at 09:45:44AM +0000, Jan Beulich wrote:
>> At least on ia64 the (bogus) use of xchg() here results in the compiler
>> warning about an unused expression result. As only an assignment is
>> intended here, convert it to such.
>> 
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> 
>> ---
>>  include/net/netfilter/xt_log.h |    2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> --- 3.3-rc6/include/net/netfilter/xt_log.h
>> +++ 3.3-rc6-netfilter-xt_log-no-xchg/include/net/netfilter/xt_log.h
>> @@ -47,7 +47,7 @@ static void sb_close(struct sbuff *m)
>>  	if (likely(m != &emergency))
>>  		kfree(m);
>>  	else {
>> -		xchg(&emergency_ptr, m);
>> +		emergency_ptr = m;
> 
> That xchg is there for some reason. I think it's better if you just
> cast it to void.

If I knew the reason, I wouldn't mind doing so. But with no code
comment and nothing said in the corresponding commit message
I'm of the opinion that it is there just as a copy-and-paste result.

Jan

Re: [PATCH] xt_log.h: don't use xchg() for simple assignment

[Eric Dumazet]

Le vendredi 23 mars 2012 à 10:14 +0000, Jan Beulich a écrit :
> >>> On 23.03.12 at 11:07, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > On Thu, Mar 08, 2012 at 09:45:44AM +0000, Jan Beulich wrote:
> >> At least on ia64 the (bogus) use of xchg() here results in the compiler
> >> warning about an unused expression result. As only an assignment is
> >> intended here, convert it to such.
> >> 
> >> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> >> 
> >> ---
> >>  include/net/netfilter/xt_log.h |    2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >> 
> >> --- 3.3-rc6/include/net/netfilter/xt_log.h
> >> +++ 3.3-rc6-netfilter-xt_log-no-xchg/include/net/netfilter/xt_log.h
> >> @@ -47,7 +47,7 @@ static void sb_close(struct sbuff *m)
> >>  	if (likely(m != &emergency))
> >>  		kfree(m);
> >>  	else {
> >> -		xchg(&emergency_ptr, m);
> >> +		emergency_ptr = m;
> > 
> > That xchg is there for some reason. I think it's better if you just
> > cast it to void.
> 
> If I knew the reason, I wouldn't mind doing so. But with no code
> comment and nothing said in the corresponding commit message
> I'm of the opinion that it is there just as a copy-and-paste result.
> 

Hmm I missed your patch Jan, and you're right, a plain write is OK here.

Acked-by: Eric Dumazet <eric.dumazet@gmail.com>



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

Re: [PATCH] xt_log.h: don't use xchg() for simple assignment

[Pablo Neira Ayuso]

On Fri, Mar 23, 2012 at 04:51:45AM -0700, Eric Dumazet wrote:
> Le vendredi 23 mars 2012 à 10:14 +0000, Jan Beulich a écrit :
> > >>> On 23.03.12 at 11:07, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > > On Thu, Mar 08, 2012 at 09:45:44AM +0000, Jan Beulich wrote:
> > >> At least on ia64 the (bogus) use of xchg() here results in the compiler
> > >> warning about an unused expression result. As only an assignment is
> > >> intended here, convert it to such.
> > >> 
> > >> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> > >> 
> > >> ---
> > >>  include/net/netfilter/xt_log.h |    2 +-
> > >>  1 file changed, 1 insertion(+), 1 deletion(-)
> > >> 
> > >> --- 3.3-rc6/include/net/netfilter/xt_log.h
> > >> +++ 3.3-rc6-netfilter-xt_log-no-xchg/include/net/netfilter/xt_log.h
> > >> @@ -47,7 +47,7 @@ static void sb_close(struct sbuff *m)
> > >>  	if (likely(m != &emergency))
> > >>  		kfree(m);
> > >>  	else {
> > >> -		xchg(&emergency_ptr, m);
> > >> +		emergency_ptr = m;
> > > 
> > > That xchg is there for some reason. I think it's better if you just
> > > cast it to void.
> > 
> > If I knew the reason, I wouldn't mind doing so. But with no code
> > comment and nothing said in the corresponding commit message
> > I'm of the opinion that it is there just as a copy-and-paste result.
> > 
> 
> Hmm I missed your patch Jan, and you're right, a plain write is OK here.
> 
> Acked-by: Eric Dumazet <eric.dumazet@gmail.com>

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