* [PATCH v3] git-compat-util.h: implement a different ARRAY_SIZE macro for for safely deriving the size of array
@ 2015-04-30 12:44 Elia Pinto
2015-04-30 17:20 ` Junio C Hamano
2015-04-30 17:41 ` Junio C Hamano
0 siblings, 2 replies; 7+ messages in thread
From: Elia Pinto @ 2015-04-30 12:44 UTC (permalink / raw)
To: git; +Cc: gitster, peff, Elia Pinto
To get number of elements in an array git use the ARRAY_SIZE macro
defined as:
#define ARRAY_SIZE(x) (sizeof(x)/sizeof((x)[0]))
The problem with it is a possibility of mistakenly passing to it a
pointer instead an array. The ARRAY_SIZE macro as conventionally
defined does not provide good type-safety and the open-coded
approach is more fragile, more verbose and provides no improvement in
type-safety.
Use instead a different but compatible ARRAY_SIZE() macro,
which will also break compile if you try to
use it on a pointer. This implemention revert to the original code
if the compiler doesn't know the typeof and __builtin_types_compatible_p
GCC extensions.
This can ensure our code is robust to changes, without
needing a gratuitous macro or constant. A similar
ARRAY_SIZE implementation also exists in the linux kernel.
Credits to Rusty Russell and his ccan library.
Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
git-compat-util.h | 54 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 53 insertions(+), 1 deletion(-)
This is the third version of the patch.
Compared to the second version:
- Eliminated the autoconf use. I use instead the GCC (and compatible compilers) macros
for checking if the not portable builtin is supported or not ("Jeff suggestion")
- Changed the name of the macro from _array_size_chk to BARF_IF_IS_NOT_AN_ARRAY i
("Junio suggestion. In ALL_CAPS for the Jeff comment )"
diff --git a/git-compat-util.h b/git-compat-util.h
index bc8fc8c..b29c9f3 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -3,6 +3,23 @@
#define _FILE_OFFSET_BITS 64
+
+/* Derived from Linux "Features Test Macro" header
+ * Convenience macros to test the versions of gcc (or
+ * a compatible compiler).
+ * Use them like this:
+ * #if GIT_GNUC_PREREQ (2,8)
+ * ... code requiring gcc 2.8 or later ...
+ * #endif
+*/
+#if defined(__GNUC__) && defined(__GNUC_MINOR__)
+# define GIT_GNUC_PREREQ(maj, min) \
+ ((__GNUC__ << 16) + __GNUC_MINOR__ >= ((maj) << 16) + (min))
+#else
+ #define GIT_GNUC_PREREQ(maj, min) 0
+#endif
+
+
#ifndef FLEX_ARRAY
/*
* See if our compiler is known to support flexible array members.
@@ -25,7 +42,42 @@
#endif
#endif
-#define ARRAY_SIZE(x) (sizeof(x)/sizeof(x[0]))
+
+/*
+ * BUILD_ASSERT_OR_ZERO - assert a build-time dependency, as an expression.
+ * @cond: the compile-time condition which must be true.
+ *
+ * Your compile will fail if the condition isn't true, or can't be evaluated
+ * by the compiler. This can be used in an expression: its value is "0".
+ *
+ * Example:
+ * #define foo_to_char(foo) \
+ * ((char *)(foo) \
+ * + BUILD_ASSERT_OR_ZERO(offsetof(struct foo, string) == 0))
+ */
+#define BUILD_ASSERT_OR_ZERO(cond) \
+ (sizeof(char [1 - 2*!(cond)]) - 1)
+
+#if defined(__GNUC__) && (__GNUC__ >= 3)
+# if GIT_GNUC_PREREQ(3, 1)
+ /* &arr[0] degrades to a pointer: a different type from an array */
+# define BARF_IF_IS_NOT_AN_ARRAY(arr) \
+ BUILD_ASSERT_OR_ZERO(!__builtin_types_compatible_p(typeof(arr), \
+ typeof(&(arr)[0])))
+# else
+# define BARF_IF_IS_NOT_AN_ARRAY(arr) 0
+# endif
+#endif
+/*
+ * ARRAY_SIZE - get the number of elements in a visible array
+ * <at> x: the array whose size you want.
+ *
+ * This does not work on pointers, or arrays declared as [], or
+ * function parameters. With correct compiler support, such usage
+ * will cause a build error (see the build_assert_or_zero macro).
+ */
+#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]) + BARF_IF_IS_NOT_AN_ARRAY(x))
+
#define bitsizeof(x) (CHAR_BIT * sizeof(x))
#define maximum_signed_value_of_type(a) \
--
2.1.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v3] git-compat-util.h: implement a different ARRAY_SIZE macro for for safely deriving the size of array
2015-04-30 12:44 [PATCH v3] git-compat-util.h: implement a different ARRAY_SIZE macro for for safely deriving the size of array Elia Pinto
@ 2015-04-30 17:20 ` Junio C Hamano
2015-04-30 17:41 ` Junio C Hamano
1 sibling, 0 replies; 7+ messages in thread
From: Junio C Hamano @ 2015-04-30 17:20 UTC (permalink / raw)
To: Elia Pinto; +Cc: git, peff
Elia Pinto <gitter.spiros@gmail.com> writes:
> To get number of elements in an array git use the ARRAY_SIZE macro
> defined as:
>
> #define ARRAY_SIZE(x) (sizeof(x)/sizeof((x)[0]))
>
> The problem with it is a possibility of mistakenly passing to it a
> pointer instead an array. The ARRAY_SIZE macro as conventionally
> defined does not provide good type-safety and the open-coded
> approach is more fragile, more verbose and provides no improvement in
> type-safety.
>
> Use instead a different but compatible ARRAY_SIZE() macro,
> which will also break compile if you try to
> use it on a pointer. This implemention revert to the original code
> if the compiler doesn't know the typeof and __builtin_types_compatible_p
> GCC extensions.
>
> This can ensure our code is robust to changes, without
> needing a gratuitous macro or constant. A similar
> ARRAY_SIZE implementation also exists in the linux kernel.
>
> Credits to Rusty Russell and his ccan library.
>
> Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
> ---
> git-compat-util.h | 54 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 53 insertions(+), 1 deletion(-)
>
> This is the third version of the patch.
>
> Compared to the second version:
>
> - Eliminated the autoconf use. I use instead the GCC (and compatible compilers) macros
> for checking if the not portable builtin is supported or not ("Jeff suggestion")
> - Changed the name of the macro from _array_size_chk to BARF_IF_IS_NOT_AN_ARRAY i
> ("Junio suggestion. In ALL_CAPS for the Jeff comment )"
Thanks. Allow me to s/BARF_IF_IS_NOT_AN_ARRAY/BARF_UNLESS_AN_ARRAY/
everywhere for brevity while applying.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3] git-compat-util.h: implement a different ARRAY_SIZE macro for for safely deriving the size of array
2015-04-30 12:44 [PATCH v3] git-compat-util.h: implement a different ARRAY_SIZE macro for for safely deriving the size of array Elia Pinto
2015-04-30 17:20 ` Junio C Hamano
@ 2015-04-30 17:41 ` Junio C Hamano
2015-04-30 17:44 ` Jeff King
1 sibling, 1 reply; 7+ messages in thread
From: Junio C Hamano @ 2015-04-30 17:41 UTC (permalink / raw)
To: Elia Pinto; +Cc: git, peff
Elia Pinto <gitter.spiros@gmail.com> writes:
> +#if defined(__GNUC__) && (__GNUC__ >= 3)
> +# if GIT_GNUC_PREREQ(3, 1)
> + /* &arr[0] degrades to a pointer: a different type from an array */
> +# define BARF_IF_IS_NOT_AN_ARRAY(arr) \
> + BUILD_ASSERT_OR_ZERO(!__builtin_types_compatible_p(typeof(arr), \
> + typeof(&(arr)[0])))
> +# else
> +# define BARF_IF_IS_NOT_AN_ARRAY(arr) 0
> +# endif
> +#endif
> +/*
> + * ARRAY_SIZE - get the number of elements in a visible array
> + * <at> x: the array whose size you want.
> + *
> + * This does not work on pointers, or arrays declared as [], or
> + * function parameters. With correct compiler support, such usage
> + * will cause a build error (see the build_assert_or_zero macro).
> + */
> +#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]) + BARF_IF_IS_NOT_AN_ARRAY(x))
> +
> #define bitsizeof(x) (CHAR_BIT * sizeof(x))
>
> #define maximum_signed_value_of_type(a) \
Hmmmmmmmm.
CC ws.o
ws.c: In function 'parse_whitespace_rule':
ws.c:46:3: error: unknown type name 'typeof'
for (i = 0; i < ARRAY_SIZE(whitespace_rule_names); i++) {
^
$ gcc --version
gcc (Ubuntu 4.8.2-19ubuntu1) 4.8.2
Copyright (C) 2013 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There
is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR
PURPOSE.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3] git-compat-util.h: implement a different ARRAY_SIZE macro for for safely deriving the size of array
2015-04-30 17:41 ` Junio C Hamano
@ 2015-04-30 17:44 ` Jeff King
2015-04-30 17:54 ` Junio C Hamano
0 siblings, 1 reply; 7+ messages in thread
From: Jeff King @ 2015-04-30 17:44 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Elia Pinto, git
On Thu, Apr 30, 2015 at 10:41:30AM -0700, Junio C Hamano wrote:
> Elia Pinto <gitter.spiros@gmail.com> writes:
>
> > +#if defined(__GNUC__) && (__GNUC__ >= 3)
> > +# if GIT_GNUC_PREREQ(3, 1)
> > + /* &arr[0] degrades to a pointer: a different type from an array */
> > +# define BARF_IF_IS_NOT_AN_ARRAY(arr) \
> > + BUILD_ASSERT_OR_ZERO(!__builtin_types_compatible_p(typeof(arr), \
> > + typeof(&(arr)[0])))
> > +# else
> > +# define BARF_IF_IS_NOT_AN_ARRAY(arr) 0
> > +# endif
> [...]
>
> Hmmmmmmmm.
>
> CC ws.o
> ws.c: In function 'parse_whitespace_rule':
> ws.c:46:3: error: unknown type name 'typeof'
> for (i = 0; i < ARRAY_SIZE(whitespace_rule_names); i++) {
> ^
>
> $ gcc --version
> gcc (Ubuntu 4.8.2-19ubuntu1) 4.8.2
> Copyright (C) 2013 Free Software Foundation, Inc.
> This is free software; see the source for copying conditions. There
> is NO
> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR
> PURPOSE.
ISTR that you compile with "-std=c89". typeof was added in c99, I think
(and was a GNU extension before that). I wonder if that is fooling the
gcc version-check.
-Peff
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3] git-compat-util.h: implement a different ARRAY_SIZE macro for for safely deriving the size of array
2015-04-30 17:44 ` Jeff King
@ 2015-04-30 17:54 ` Junio C Hamano
2015-04-30 17:59 ` Junio C Hamano
0 siblings, 1 reply; 7+ messages in thread
From: Junio C Hamano @ 2015-04-30 17:54 UTC (permalink / raw)
To: Jeff King; +Cc: Elia Pinto, git
Jeff King <peff@peff.net> writes:
> ISTR that you compile with "-std=c89". typeof was added in c99, I think
> (and was a GNU extension before that). I wonder if that is fooling the
> gcc version-check.
Yeah, I think that is very likely.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3] git-compat-util.h: implement a different ARRAY_SIZE macro for for safely deriving the size of array
2015-04-30 17:54 ` Junio C Hamano
@ 2015-04-30 17:59 ` Junio C Hamano
2015-04-30 18:19 ` Jeff King
0 siblings, 1 reply; 7+ messages in thread
From: Junio C Hamano @ 2015-04-30 17:59 UTC (permalink / raw)
To: Jeff King; +Cc: Elia Pinto, git
Junio C Hamano <gitster@pobox.com> writes:
> Jeff King <peff@peff.net> writes:
>
>> ISTR that you compile with "-std=c89". typeof was added in c99, I think
>> (and was a GNU extension before that). I wonder if that is fooling the
>> gcc version-check.
>
> Yeah, I think that is very likely.
Yes. One workaround is to explicitly say that we accept the GNU
extension in the source, perhaps.
git-compat-util.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/git-compat-util.h b/git-compat-util.h
index f1f8f23..7fad5aa 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -62,8 +62,8 @@
# if GIT_GNUC_PREREQ(3, 1)
/* &arr[0] degrades to a pointer: a different type from an array */
# define BARF_UNLESS_AN_ARRAY(arr) \
- BUILD_ASSERT_OR_ZERO(!__builtin_types_compatible_p(typeof(arr), \
- typeof(&(arr)[0])))
+ BUILD_ASSERT_OR_ZERO(!__builtin_types_compatible_p(__typeof__(arr), \
+ __typeof__(&(arr)[0])))
# else
# define BARF_UNLESS_AN_ARRAY(arr) 0
# endif
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v3] git-compat-util.h: implement a different ARRAY_SIZE macro for for safely deriving the size of array
2015-04-30 17:59 ` Junio C Hamano
@ 2015-04-30 18:19 ` Jeff King
0 siblings, 0 replies; 7+ messages in thread
From: Jeff King @ 2015-04-30 18:19 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Elia Pinto, git
On Thu, Apr 30, 2015 at 10:59:06AM -0700, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
> > Jeff King <peff@peff.net> writes:
> >
> >> ISTR that you compile with "-std=c89". typeof was added in c99, I think
> >> (and was a GNU extension before that). I wonder if that is fooling the
> >> gcc version-check.
> >
> > Yeah, I think that is very likely.
>
> Yes. One workaround is to explicitly say that we accept the GNU
> extension in the source, perhaps.
>
> git-compat-util.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/git-compat-util.h b/git-compat-util.h
> index f1f8f23..7fad5aa 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -62,8 +62,8 @@
> # if GIT_GNUC_PREREQ(3, 1)
> /* &arr[0] degrades to a pointer: a different type from an array */
> # define BARF_UNLESS_AN_ARRAY(arr) \
> - BUILD_ASSERT_OR_ZERO(!__builtin_types_compatible_p(typeof(arr), \
> - typeof(&(arr)[0])))
> + BUILD_ASSERT_OR_ZERO(!__builtin_types_compatible_p(__typeof__(arr), \
> + __typeof__(&(arr)[0])))
That seems like a reasonable fix, provided that other gcc-compatible
compilers implement the GNU-compatible name. I just tried with clang
3.5, and it seems to work fine.
And I was wrong about "typeof". It is not in C99 at all. So I think it
is really a matter of whether you are in the default "-std=gnu89" mode
or not, as to whether the compiler imports the "typeof" keyword, or if
it must be hidden in the compiler-reserved area (starting with "__").
-Peff
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2015-04-30 18:19 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-30 12:44 [PATCH v3] git-compat-util.h: implement a different ARRAY_SIZE macro for for safely deriving the size of array Elia Pinto
2015-04-30 17:20 ` Junio C Hamano
2015-04-30 17:41 ` Junio C Hamano
2015-04-30 17:44 ` Jeff King
2015-04-30 17:54 ` Junio C Hamano
2015-04-30 17:59 ` Junio C Hamano
2015-04-30 18:19 ` Jeff King
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.