public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed

From: Junio C Hamano <gitster@pobox•com>
To: Elia Pinto <gitter.spiros@gmail•com>
Cc: git@vger•kernel.org, peff@peff•net
Subject: Re: [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 10:20:11 -0700	[thread overview]
Message-ID: <xmqqk2wt1qk4.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <1430397854-28908-1-git-send-email-gitter.spiros@gmail.com> (Elia Pinto's message of "Thu, 30 Apr 2015 14:44:14 +0200")

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.

next prev parent reply	other threads:[~2015-04-30 17:20 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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=xmqqk2wt1qk4.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=gitter.spiros@gmail$(echo .)com \
    --cc=peff@peff$(echo .)net \
    /path/to/YOUR_REPLY

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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox