git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "Björn Steinbrink" <B.Steinbrink@gmx.de>
Cc: gitster@pobox.com, raa.lkml@gmail.com, git@ostkamp.fastmail.fm,
	git@vger.kernel.org
Subject: Re: [PATCH] Fix "identifier redeclared" compilation error with SUN cc.
Date: Thu, 15 Nov 2007 20:58:55 -0800	[thread overview]
Message-ID: <7vd4ua3hww.fsf@gitster.siamese.dyndns.org> (raw)
In-Reply-To: <1195089303-28085-1-git-send-email-B.Steinbrink@gmx.de> (Björn Steinbrink's message of "Thu, 15 Nov 2007 02:15:03 +0100")

Björn Steinbrink <B.Steinbrink@gmx.de> writes:

> Guido, could you please test this patch?
>
> I have no clue which versions of SUN's cc are affected, so I simply enabled
> the workaround for all versions. Someone with more knowledge about that
> should probably limit the check to only do that for the broken versions.
>
>  git-compat-util.h |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)

> diff --git a/git-compat-util.h b/git-compat-util.h
> index ede9408..c3ff4b4 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -6,6 +6,8 @@
>  #ifndef FLEX_ARRAY
>  #if defined(__GNUC__) && (__GNUC__ < 3)
>  #define FLEX_ARRAY 0
> +#elif defined(sun) || defined(__SUN__)
> +#define FLEX_ARRAY 1
>  #else
>  #define FLEX_ARRAY /* empty */
>  #endif

This feels a bit too narrow and too broad at the same time,
doesn't it?

As I suspect there are other compilers that do not implement
flexible array members (so you cannot use "member[]") nor older
gcc extension of zero sized member (so you cannot use
"member[0]" either), this checking specifically for Sun is too
narrow.

On the other hand, as you said, this is too broad, because not
everybody may be using the SUN compiler on Sun, nor the version
that does not understand flexible array members.

But being broad should always be safer, albeit a bit wasteful.

How about doing it this way?

  # ifndef FLEX_ARRAY
  #   if defined(__GNUC__)
  #     if (__GNUC__ < 3)
  #       define FLEX_ARRAY 0
  #     else
  #       define FLEX_ARRAY /* empty */
  #     endif
  #   else
        /* more cases we know we can use 0 or empty can come here */
  #   endif
  # endif

  /* if still undefined, default to the safe, old fashioned way */
  # ifndef FLEX_ARRAY
  #   define FLEX_ARRAY 1
  # endif

The basic idea is:

 * The user (from Makefile command line, config.mak, or you
   could add autoconf test) can pass -DFLEX_ARRAY=... to specify
   exactly what should happen;

 * Otherwise, if we happen to know for sure that we can use "0"
   or "/* empty */" with the compiler, we define FLEX_ARRAY;
   currently we know such things for gcc.

 * For everybody else, we use safer default of "1".  IOW, if you
   know your compiler does not grok "/* empty */" nor "0", you
   do not have to do anything special but use the default case
   as everybody else.

  parent reply	other threads:[~2007-11-16  4:59 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-11-14 20:31 [PATCH] Fix Solaris Workshop Compiler issues Guido Ostkamp
2007-11-14 20:47 ` Alex Riesen
2007-11-14 21:25   ` Junio C Hamano
2007-11-14 23:21     ` Guido Ostkamp
2007-11-14 23:28       ` Alex Riesen
2007-11-15  0:17         ` Björn Steinbrink
2007-11-15  0:30           ` Junio C Hamano
2007-11-15  0:44             ` Björn Steinbrink
2007-11-15  0:46               ` Junio C Hamano
2007-11-15  0:50                 ` Björn Steinbrink
2007-11-15  1:15                 ` [PATCH] Fix "identifier redeclared" compilation error with SUN cc Björn Steinbrink
2007-11-15 22:00                   ` Guido Ostkamp
2007-11-15 22:15                     ` Junio C Hamano
2007-11-15 22:28                       ` Guido Ostkamp
2007-11-16 18:59                       ` [PATCH] Add mkdtemp() workaround for Sun Solaris 10 Guido Ostkamp
2007-11-17  0:33                         ` [RFH] Solaris portability Junio C Hamano
2007-11-18 12:08                           ` Guido Ostkamp
2007-11-18 17:46                             ` Junio C Hamano
2007-11-16  4:58                   ` Junio C Hamano [this message]
2007-11-16 12:55                     ` [PATCH] Fix "identifier redeclared" compilation error with SUN cc Björn Steinbrink
2007-11-19 17:51                     ` Guido Ostkamp
2007-11-20  8:30                       ` Junio C Hamano
2007-11-20 17:28                         ` Guido Ostkamp
2007-11-20 18:06                           ` Guido Ostkamp
2007-11-20 18:26                       ` Martin Mares
2007-11-20 20:08                         ` Junio C Hamano
2007-11-20 20:09                           ` Martin Mares
2007-11-15  0:44             ` [PATCH] Fix Solaris Workshop Compiler issues Linus Torvalds
2007-11-15  1:21               ` David Kastrup
2007-11-15  1:53                 ` Linus Torvalds
2007-11-15  3:27                 ` Junio C Hamano

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=7vd4ua3hww.fsf@gitster.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=B.Steinbrink@gmx.de \
    --cc=git@ostkamp.fastmail.fm \
    --cc=git@vger.kernel.org \
    --cc=raa.lkml@gmail.com \
    /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).