[ovs-dev] [PATCH] netdev-linux: Avoid "cleverness" in swap_uint64().

Justin Pettit jpettit at nicira.com
Mon Apr 11 06:17:39 UTC 2011


Looks good to me.

--Justin


On Apr 8, 2011, at 4:45 PM, Ben Pfaff wrote:

> Obviously correct code is easier on everyone.  As the C FAQ says:
> 
> 20.15c:	How can I swap two values without using a temporary?
> 
> A:	The standard hoary old assembly language programmer's trick is:
> 
> 		a ^= b;
> 		b ^= a;
> 		a ^= b;
> 
> 	But this sort of code has little place in modern, HLL
> 	programming.  Temporary variables are essentially free,
> 	and the idiomatic code using three assignments, namely
> 
> 		int t = a;
> 		a = b;
> 		b = t;
> 
> 	is not only clearer to the human reader, it is more likely to be
> 	recognized by the compiler and turned into the most-efficient
> 	code (e.g. using a swap instruction, if available).  The latter
> 	code is obviously also amenable to use with pointers and
> 	floating-point values, unlike the XOR trick.  See also questions
> 	3.3b and 10.3.
> ---
> lib/netdev-linux.c |    6 +++---
> 1 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
> index eecaaa5..1aa9093 100644
> --- a/lib/netdev-linux.c
> +++ b/lib/netdev-linux.c
> @@ -1144,9 +1144,9 @@ netdev_linux_update_is_pseudo(struct netdev_dev_linux *netdev_dev)
> static void
> swap_uint64(uint64_t *a, uint64_t *b)
> {
> -    *a ^= *b;
> -    *b ^= *a;
> -    *a ^= *b;
> +    uint64_t tmp = *a;
> +    *a = *b;
> +    *b = tmp;
> }
> 
> /* Retrieves current device stats for 'netdev'. */
> -- 
> 1.7.1
> 
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev




More information about the dev mailing list