On 04/19, Eric Wong wrote:
> Johannes Sixt <j6t@xxxxxxxx> wrote:
> > Am 19.04.2017 um 01:18 schrieb Brandon Williams:
> > >@@ -400,6 +404,53 @@ static char **prep_childenv(const char *const *deltaenv)
> > > }
> > > #endif
> > >
> >
> > Does this #endif in this hunk context belong to an #ifndef
> > GIT_WINDOWS_NATIVE? If so, I wonder why these new functions are outside
> > these brackets? An oversight?
>
> Seems like an oversight, sorry about that.
> All the new atfork stuff I added should be protected by
> #ifndef GIT_WINDOWS_NATIVE.
>
> Brandon / Johannes: can you fixup on your end?
Correct, this is an oversight I should have caught :)
No worries though, I'll fix it up in a reroll (since I'm going to be
need to send out another version to fix up another patch in the series
for Windows)
>
> I wonder if some of this OS-specific code would be more
> easily maintained if split out further to OS-specific files,
> even at the risk of some code duplication.
>
> And/or perhaps label all #else and #endif statements with
> comments, and limit the scope of each ifdef block to be
> per-function for with tiny attention spans like me :x
Yeah I'm not sure I know the best way to prevent this from happening,
thankfully we have windows folk who keep us honest :D
>
> > >+struct atfork_state {
> > >+#ifndef NO_PTHREADS
> > >+ int cs;
> > >+#endif
> > >+ sigset_t old;
> > >+};
> > ...
> >
> > -- Hannes
> >
--
Brandon Williams