Re: [PATCH] daemon: restore getpeername(0,...) use

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]