On Sun, Aug 17, 2014 at 09:46:42AM +0200, Johannes Sixt wrote:
> This is a step in the right direction, IMO. This way to initialize the
> struct feels mucth better because it does not depend on that the bit
> pattern of the NULL pointer is all zeros.
I think platforms with NULL as something besides all-bits-zero are a
lost cause with git. There are so many struct memsets that depend on
this (and it's probably not actually worth caring about).
> > +#define CHILD_PROCESS_INIT { NULL }
>
> I would have expected this to read
>
> #define CHILD_PROCESS_INIT { NULL, ARGV_ARRAY_INIT }
>
> It does change the bit pattern of the initialized struct child_process
> because ARGV_ARRAY_INIT uses a non-NULL address. But IMHO
> ARGV_ARRAY_INIT should be used here as a defensive measure.
I'd be OK with that. The argv_array code is specifically OK with an
all-bits-zero initialization. The only thing you don't get is that an
empty array is non-NULL, but that should never matter here (true, we'd
segfault if you didn't add anything to the array, but that is clearly a
bug that needs to be fixed either way).
I'm a little worried, though, that use sites without initializers would
be left behind. For example, git_proxy_connect uses xcalloc to allocate
the child_process, which results in all-bits-zero. If we want to start
caring about the initialization, we probably need to provide a
child_process_init() function and use it consistently.
-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