generic swap()

generic swap()

[Peter Zijlstra]

Hi,

Where did this patch come from? -- I can't seem to find it on lkml at all..

The reason I ask it that I wonder why swap() has a return value?

---

commit 91f68b7359144aa40bb9668124543d15284750b4
Author: Wu Fengguang <fengguang.wu@intel.com>
Date:   Wed Jan 7 18:09:12 2009 -0800

    generic swap(): introduce global macro swap(a, b)
    
    There have been some local definitions of swap(), it's time to replace
    them all with a uniform one.
    
    Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
    Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
    Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 6b8e202..343df9e 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -476,6 +476,12 @@ static inline char *pack_hex_byte(char *buf, u8 byte)
 	__val = __val < __min ? __min: __val;	\
 	__val > __max ? __max: __val; })
 
+
+/*
+ * swap - swap value of @a and @b
+ */
+#define swap(a, b) ({ typeof(a) __tmp = (a); (a) = (b); (b) = __tmp; })
+
 /**
  * container_of - cast a member of a structure out to the containing structure
  * @ptr:	the pointer to the member.

Re: generic swap()

[Andrew Morton]

On Fri, 16 Jan 2009 10:26:34 +0100 Peter Zijlstra <peterz@infradead.org> wrote:

> Hi,
> 
> Where did this patch come from? -- I can't seem to find it on lkml at all..

That's a damn good question.  I received:

From: Wu Fengguang <fengguang.wu@intel.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: LKML <linux-kernel@vger.kernel.org>
Subject: [PATCH 0/9] [PATCH] make swap() a global macro
Date: Tue, 02 Dec 2008 19:45:24 +0800
User-Agent: quilt/0.46-1

but the cc didn't seem to work.  quilt bustage?

> The reason I ask it that I wonder why swap() has a return value?

Accident, I guess.

> +#define swap(a, b) ({ typeof(a) __tmp = (a); (a) = (b); (b) = __tmp; })

You think it should use the do{}while(0) thing?

Re: generic swap()

[Peter Zijlstra]

On Fri, 2009-01-16 at 01:38 -0800, Andrew Morton wrote:
> On Fri, 16 Jan 2009 10:26:34 +0100 Peter Zijlstra <peterz@infradead.org> wrote:
> 
> > Hi,
> > 
> > Where did this patch come from? -- I can't seem to find it on lkml at all..
> 
> That's a damn good question.  I received:

Hehe, one of the mysteries of life then :-)

> > The reason I ask it that I wonder why swap() has a return value?
> 
> Accident, I guess.
> 
> > +#define swap(a, b) ({ typeof(a) __tmp = (a); (a) = (b); (b) = __tmp; })
> 
> You think it should use the do{}while(0) thing?

That was what I was thinking indeed. Non of the current users appear to
make use of the (somewhat arbitrary) return value, so lets change it
before someone gets creative ;-)

Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
---
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 343df9e..7fa3718 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -480,7 +480,8 @@ static inline char *pack_hex_byte(char *buf, u8 byte)
 /*
  * swap - swap value of @a and @b
  */
-#define swap(a, b) ({ typeof(a) __tmp = (a); (a) = (b); (b) = __tmp; })
+#define swap(a, b) \
+	do { typeof(a) __tmp = (a); (a) = (b); (b) = __tmp; } while (0)
 
 /**
  * container_of - cast a member of a structure out to the containing structure

Re: generic swap()

[Wu Fengguang]

On Fri, Jan 16, 2009 at 10:45:36AM +0100, Peter Zijlstra wrote:
> On Fri, 2009-01-16 at 01:38 -0800, Andrew Morton wrote:
> > On Fri, 16 Jan 2009 10:26:34 +0100 Peter Zijlstra <peterz@infradead.org> wrote:
> > 
> > > Hi,
> > > 
> > > Where did this patch come from? -- I can't seem to find it on lkml at all..
> > 
> > That's a damn good question.  I received:
> 
> Hehe, one of the mysteries of life then :-)
> 
> > > The reason I ask it that I wonder why swap() has a return value?
> > 
> > Accident, I guess.
> > 
> > > +#define swap(a, b) ({ typeof(a) __tmp = (a); (a) = (b); (b) = __tmp; })
> > 
> > You think it should use the do{}while(0) thing?
> 
> That was what I was thinking indeed. Non of the current users appear to
> make use of the (somewhat arbitrary) return value, so lets change it
> before someone gets creative ;-)
> 
> Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>

Acked-by: Wu Fengguang <fengguang.wu@intel.com>

Good change, thank you Peter.

//Sorry I was not able to read this email because of VPN issues.

Fengguang

> ---
> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> index 343df9e..7fa3718 100644
> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h
> @@ -480,7 +480,8 @@ static inline char *pack_hex_byte(char *buf, u8 byte)
>  /*
>   * swap - swap value of @a and @b
>   */
> -#define swap(a, b) ({ typeof(a) __tmp = (a); (a) = (b); (b) = __tmp; })
> +#define swap(a, b) \
> +	do { typeof(a) __tmp = (a); (a) = (b); (b) = __tmp; } while (0)
>  
>  /**
>   * container_of - cast a member of a structure out to the containing structure
>