git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Elia Pinto <gitter.spiros@gmail.com>
To: git@vger.kernel.org
Cc: gitster@pobox.com, peff@peff.net, Elia Pinto <gitter.spiros@gmail.com>
Subject: [PATCH v3] git-compat-util.h: implement a different ARRAY_SIZE macro for for safely deriving the size of array
Date: Thu, 30 Apr 2015 14:44:14 +0200	[thread overview]
Message-ID: <1430397854-28908-1-git-send-email-gitter.spiros@gmail.com> (raw)

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

             reply	other threads:[~2015-04-30 12:44 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-30 12:44 Elia Pinto [this message]
2015-04-30 17:20 ` [PATCH v3] git-compat-util.h: implement a different ARRAY_SIZE macro for for safely deriving the size of array 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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1430397854-28908-1-git-send-email-gitter.spiros@gmail.com \
    --to=gitter.spiros@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).