Patchwork xt_log.h: don't use xchg() for simple assignment

login
register
mail settings
Submitter Jan Beulich
Date March 8, 2012, 9:45 a.m.
Message ID <4F588DD8020000780007711C@nat28.tlf.novell.com>
Download mbox | patch
Permalink /patch/145475/
State Accepted
Headers show

Comments

Jan Beulich - March 8, 2012, 9:45 a.m.
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(-)




--
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
Pablo Neira - March 23, 2012, 10:07 a.m.
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.
--
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
Jan Beulich - March 23, 2012, 10:14 a.m.
>>> 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

--
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
Eric Dumazet - March 23, 2012, 11:51 a.m.
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
Pablo Neira - March 26, 2012, 12:18 p.m.
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

Patch

--- 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();
 	}
 }