Web lists-archives.com

Re: [PATCH v5 11/11] run-command: block signals between fork and execve




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