All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Joachim Schmitz" <jojo@schmitz-digital.de>
To: git@vger.kernel.org
Subject: Re: [PATCH] daemon: restore getpeername(0,...) use
Date: Mon, 10 Sep 2012 19:26:26 +0200	[thread overview]
Message-ID: <k2l7s5$gl9$1@ger.gmane.org> (raw)
In-Reply-To: 20120910155006.GA8737@sigill.intra.peff.net

Jeff King wrote:
> On Mon, Sep 10, 2012 at 04:38:58PM +0200, Joachim Schmitz wrote:
>
>>> More importantly, though, is it actually portable? I thought it was
>>> added in C99, and we try to stick to C89 to support older compilers
>>> and systems. My copy of C99 is vague (it says only that the "bool"
>>> macro was added via stdbool.h in C99, but nothing about the "true"
>>> and "false" macros), and I don't have a copy of C89 handy.
>>> Wikipedia does claim the header wasn't standardized at all until
>>> C99:
>>>
>>> https://en.wikipedia.org/wiki/C_standard_library
>>
>> Indeed stdbool is not part of C89, but inline isn't either and used
>> extensively in git (could possible be defined away),
>
> You can define INLINE in the Makefile to disable it (or set it to
> something more appropriate for your system).

That's what I meant.

>> as are non-const array intializers, e.g.:
>>
>>                const char *args[] = { editor, path, NULL };
>>                                               ^
>> ".../git/editor.c", line 39: error(122): expression must have a
>> constant value
>>
>> So git source is not plain C89 code (anymore?)
>
> I remember we excised a whole bunch of non-constant initializers at
> some point because somebody's compiler was complaining. But I suppose
> this one has slipped back in, because non-constant initializers are
> so damn useful. And nobody has complained, which I imagine means
> nobody has bothered building lately on those older systems that
> complained.

OK, record my complaint then ;-)
At least some older release of HP NonStop only have C89 and are still in use

And tying to compile in plain C89 mode revealed several other problems too 
(e.g. size_t seems not to be typedef'd?)

> My "we stick to C89" is a little bit of a lie. We do not care about
> specific standards. We do care about running everywhere on reasonable
> systems. So something that is C99 might be OK if realistically
> everybody has it. And something that is POSIX is not automatically OK
> if there are many real-world systems that do not have it.
>
> Since there is no written standard, there tends to be an organic ebb
> and flow in which features we use. Somebody will use a feature that
> is not portable because it's useful to them, and then somebody whose
> system will no longer build git will complain, and then we'll fix it
> and move on. As reviewers, we try to anticipate those breakages and
> stop them early (especially if they are ones we have seen before and
> know are a problem), but we do not always get it right. And sometimes
> it is even time to revisit old decisions (the line you mentioned is 2
> years old, and nobody has complained; maybe all of the old systems
> have become obsolete, and we no longer need to care about constant
> initializers).
>
> Getting back to the patch at hand, it may be that stdbool is
> practically available everywhere. Or that we could trivially emulate
> it by defining a "true" macro in this case. But it is also important
> to consider whether that complexity is worth it. This would be the
> first and only spot in git to use "true". Why bother?
>
> -Peff 

  reply	other threads:[~2012-09-10 17:26 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-09-08 17:09 Restore hostname logging in inetd mode Jan Engelhardt
2012-09-08 17:09 ` [PATCH] daemon: restore getpeername(0,...) use Jan Engelhardt
2012-09-08 17:57   ` Joachim Schmitz
2012-09-08 19:03     ` Junio C Hamano
2012-09-08 19:20       ` Joachim Schmitz
2012-09-08 18:59   ` Junio C Hamano
2012-09-08 19:20     ` Jan Engelhardt
2012-09-10 14:21       ` Jeff King
2012-09-10 14:38         ` Joachim Schmitz
2012-09-10 15:50           ` Jeff King
2012-09-10 17:26             ` Joachim Schmitz [this message]
2012-09-10 17:58               ` Jeff King
2012-09-10 18:27                 ` Joachim Schmitz
2012-09-10 20:15                   ` Jeff King
2012-09-08 18:57 ` Restore hostname logging in inetd mode Junio C Hamano
2012-09-08 19:18   ` Jan Engelhardt

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='k2l7s5$gl9$1@ger.gmane.org' \
    --to=jojo@schmitz-digital.de \
    --cc=git@vger.kernel.org \
    /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 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.