[ovs-dev] [PATCH] unaligned: Make get_unaligned_be64() compatible on GCC and non-GCC.

Ben Pfaff blp at nicira.com
Thu Oct 9 15:37:50 UTC 2014


Thanks, applied to master.

On Thu, Oct 09, 2014 at 02:49:01PM +0000, Alin Serdean wrote:
> Tested-by: Alin Gabriel Serdean <aserdean at cloudbasesolutions.com>
> Acked-by: Alin Gabriel Serdean <aserdean at cloudbasesolutions.com>
> 
> 
> -----Mesaj original-----
> De la: Ben Pfaff [mailto:blp at nicira.com] 
> Trimis: Thursday, October 9, 2014 8:14 AM
> Către: dev at openvswitch.org
> Cc: Ben Pfaff; Alin Serdean
> Subiect: [PATCH] unaligned: Make get_unaligned_be64() compatible on GCC and non-GCC.
> 
> Until now, with GCC, get_unaligned_be64() had an interface that accepted a "ovs_be64 *", and with other compilers its accepted any pointer-to-64-bit type, but not void *.  This commit fixes the problem, making the interface the same in both cases.
> 
> This fixes a build error on MSVC:
> 
>     lib/nx-match.c(320) : error C2100: illegal indirection
>     lib/nx-match.c(320) : error C2034: 'build_assert_failed' : type of bit
>         field too small for number of bits
>     lib/nx-match.c(320) : error C2296: '%' : illegal, left operand has
>         type 'void *'
>     lib/nx-match.c(320) : error C2198: 'ntohll' : too few arguments for call
> 
> It might appear that this patch changes get_unaligned_u64() but in fact it onloy moves it earlier in the file (since it is now called from the non-GCC fork of the #if).
> 
> Reported-by: Alin Serdean <aserdean at cloudbasesolutions.com>
> Signed-off-by: Ben Pfaff <blp at nicira.com>
> ---
>  lib/unaligned.h | 58 ++++++++++++++++++++++++++++++++++-----------------------
>  1 file changed, 35 insertions(+), 23 deletions(-)
> 
> diff --git a/lib/unaligned.h b/lib/unaligned.h index 154eb13..d56f22b 100644
> --- a/lib/unaligned.h
> +++ b/lib/unaligned.h
> @@ -1,5 +1,5 @@
>  /*
> - * Copyright (c) 2010, 2011 Nicira, Inc.
> + * Copyright (c) 2010, 2011, 2014 Nicira, Inc.
>   *
>   * Licensed under the Apache License, Version 2.0 (the "License");
>   * you may not use this file except in compliance with the License.
> @@ -37,6 +37,27 @@ static inline void put_unaligned_be16(ovs_be16 *, ovs_be16);  static inline void put_unaligned_be32(ovs_be32 *, ovs_be32);  static inline void put_unaligned_be64(ovs_be64 *, ovs_be64);
>  
> +/* uint64_t get_unaligned_u64(uint64_t *p);
> + *
> + * Returns the value of the possibly misaligned uint64_t at 'p'.  'p' 
> +may
> + * actually be any type that points to a 64-bit integer.  That is, on 
> +Unix-like
> + * 32-bit ABIs, it may point to an "unsigned long long int", and on 
> +Unix-like
> + * 64-bit ABIs, it may point to an "unsigned long int" or an "unsigned 
> +long
> + * long int".
> + *
> + * This is special-cased because on some Linux targets, the kernel 
> +__u64 is
> + * unsigned long long int and the userspace uint64_t is unsigned long 
> +int, so
> + * that any single function prototype would fail to accept one or the other.
> + *
> + * Below, "sizeof (*(P) % 1)" verifies that *P has an integer type, 
> +since
> + * operands to % must be integers.
> + */
> +#define get_unaligned_u64(P)                                \
> +    (BUILD_ASSERT(sizeof *(P) == 8),                        \
> +     BUILD_ASSERT_GCCONLY(!TYPE_IS_SIGNED(typeof(*(P)))),   \
> +     (void) sizeof (*(P) % 1),                              \
> +     get_unaligned_u64__((const uint64_t *) (P)))
> +
>  #ifdef __GNUC__
>  /* GCC implementations. */
>  #define GCC_UNALIGNED_ACCESSORS(TYPE, ABBREV)   \
> @@ -136,32 +157,23 @@ static inline void put_unaligned_u64__(uint64_t *p_, uint64_t x_)
>   * accessors. */
>  #define get_unaligned_be16 get_unaligned_u16  #define get_unaligned_be32 get_unaligned_u32 -#define get_unaligned_be64 get_unaligned_u64  #define put_unaligned_be16 put_unaligned_u16  #define put_unaligned_be32 put_unaligned_u32  #define put_unaligned_be64 put_unaligned_u64 -#endif
>  
> -/* uint64_t get_unaligned_u64(uint64_t *p);
> - *
> - * Returns the value of the possibly misaligned uint64_t at 'p'.  'p' may
> - * actually be any type that points to a 64-bit integer.  That is, on Unix-like
> - * 32-bit ABIs, it may point to an "unsigned long long int", and on Unix-like
> - * 64-bit ABIs, it may point to an "unsigned long int" or an "unsigned long
> - * long int".
> - *
> - * This is special-cased because on some Linux targets, the kernel __u64 is
> - * unsigned long long int and the userspace uint64_t is unsigned long int, so
> - * that any single function prototype would fail to accept one or the other.
> - *
> - * Below, "sizeof (*(P) % 1)" verifies that *P has an integer type, since
> - * operands to % must be integers.
> - */
> -#define get_unaligned_u64(P)                                \
> -    (BUILD_ASSERT(sizeof *(P) == 8),                        \
> -     BUILD_ASSERT_GCCONLY(!TYPE_IS_SIGNED(typeof(*(P)))),   \
> -     (void) sizeof (*(P) % 1),                              \
> -     get_unaligned_u64__((const uint64_t *) (P)))
> +/* We do not #define get_unaligned_be64 as for the other be<N> 
> +functions above,
> + * because such a definition would mean that get_unaligned_be64() would 
> +have a
> + * different interface in each branch of the #if: with GCC it would 
> +take a
> + * "ovs_be64 *", with other compilers any pointer-to-64-bit-type (but 
> +not void
> + * *).  The latter means code like "get_unaligned_be64(ofpbuf_data(b))" 
> +would
> + * work with GCC but not with other compilers, which is surprising and
> + * undesirable.  Hence this wrapper function. */ static inline ovs_be64 
> +get_unaligned_be64(const ovs_be64 *p) {
> +    return get_unaligned_u64(p);
> +}
> +#endif
>  
>  /* Stores 'x' at possibly misaligned address 'p'.
>   *
> --
> 2.1.0
> 



More information about the dev mailing list