On Wed, Oct 02, 2013 at 12:34:53PM -0700, Ben Pfaff wrote:
> On Wed, Oct 02, 2013 at 02:40:09AM -0300, Flavio Leitner wrote:
> > GCC provides two useful builtin functions that can help
> > to improve array size checking during compilation.
> >
> > This patch contains no functional changes, but it makes
> > it easier to detect mistakes.
> >
> > Signed-off-by: Flavio Leitner <[email protected]>
>
> This seems simpler:
>
> --8<--------------------------cut here-------------------------->8--
>
> From: Flavio Leitner <[email protected]>
> Date: Wed, 2 Oct 2013 02:40:09 -0300
> Subject: [PATCH] util: use gcc builtins to better check array sizes
>
> GCC provides two useful builtin functions that can help
> to improve array size checking during compilation.
>
> This patch contains no functional changes, but it makes
> it easier to detect mistakes.
>
> Signed-off-by: Flavio Leitner <[email protected]>
> Signed-off-by: Ben Pfaff <[email protected]>
> ---
> lib/util.h | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/lib/util.h b/lib/util.h
> index 0db41be..8978bdb 100644
> --- a/lib/util.h
> +++ b/lib/util.h
> @@ -87,8 +87,18 @@ void ovs_assert_failure(const char *, const char *, const
> char *) NO_RETURN;
>
> extern const char *program_name;
>
> +#ifdef __GNUC__
> +/* Fails a build assertion if ARRAY is not an array type. */
> +#define ARRAY_CHECK(ARRAY) \
> + BUILD_ASSERT(!__builtin_types_compatible_p(typeof(ARRAY), \
> + typeof(&ARRAY[0])))
I thought about using the BUILD_ASSERT(), but it doesn't seem to
work in all contexts that ARRAY_SIZE() is being used, so I wrote
those new macros.
Did it work for you? If so, in which branch? I am on master and it
breaks here:
gcc -DHAVE_CONFIG_H -I. -I ./include -I ./lib -I ./lib
-Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith
-Wdeclaration-after-statement -Wswitch-enum -Wunused-parameter
-Wstrict-aliasing -Wbad-function-cast -Wcast-align -Wmissing-prototypes
-Wmissing-field-initializers -g -O2 -MT lib/ofp-util.o -MD -MP -MF
$depbase.Tpo -c -o lib/ofp-util.o lib/ofp-util.c &&\
mv -f $depbase.Tpo $depbase.Po
In file included from lib/hash.h:23:0,
from lib/flow.h:27,
from lib/classifier.h:105,
from lib/ofp-util.c:28:
lib/util.h:45:23: error: initializer element is not constant
sizeof(struct { unsigned int build_assert_failed : (EXPR) ? 1 :
-1; })
^
lib/util.h:48:35: note: in expansion of macro ‘BUILD_ASSERT__’
#define BUILD_ASSERT(EXPR) (void) BUILD_ASSERT__(EXPR)
^
lib/util.h:93:5: note: in expansion of macro ‘BUILD_ASSERT’
BUILD_ASSERT(!__builtin_types_compatible_p(typeof(ARRAY),
\
^
lib/util.h:101:6: note: in expansion of macro ‘ARRAY_CHECK’
(ARRAY_CHECK(ARRAY), sizeof(ARRAY) / sizeof((ARRAY)[0]))
^
lib/ofp-util.c:657:40: note: in expansion of macro ‘ARRAY_SIZE’
size_t ofputil_n_flow_dump_protocols =
ARRAY_SIZE(ofputil_flow_dump_protocols);
But the code looks correct.
The nice thing about using __builtin_choose_expr() is that
it doesn't evaluate the expression that was not chosen.
Thanks,
fbl
> +#else
> +#define ARRAY_CHECK(ARRAY) ((void) 0)
> +#endif
> +
> /* Returns the number of elements in ARRAY. */
> -#define ARRAY_SIZE(ARRAY) (sizeof ARRAY / sizeof *ARRAY)
> +#define ARRAY_SIZE(ARRAY) \
> + (ARRAY_CHECK(ARRAY), sizeof(ARRAY) / sizeof((ARRAY)[0]))
>
> /* Returns X / Y, rounding up. X must be nonnegative to round correctly. */
> #define DIV_ROUND_UP(X, Y) (((X) + ((Y) - 1)) / (Y))
> --
> 1.7.10.4
>
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
