Re: [PATCH 80/83] run-command: make dup_devnull() non static

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi Chris,

On Sat, 7 May 2016, Christian Couder wrote:

> On Fri, May 6, 2016 at 5:34 PM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
> >
> > On Fri, 6 May 2016, Christian Couder wrote:
> >
> >> On Thu, May 5, 2016 at 10:07 PM, Johannes Sixt <j6t@kdbg.org> wrote:
> >> > Am 05.05.2016 um 11:50 schrieb Christian Couder:
> >> >>
> >> >> On Mon, Apr 25, 2016 at 5:05 PM, Johannes Schindelin
> >> >> <Johannes.Schindelin@gmx.de> wrote:
> >> >>>
> >> >>> Hi Chris,
> >> >>>
> >> >>> On Sun, 24 Apr 2016, Christian Couder wrote:
> >> >>>
> >> >>>> diff --git a/run-command.c b/run-command.c
> >> >>>> index 8c7115a..29d2bda 100644
> >> >>>> --- a/run-command.c
> >> >>>> +++ b/run-command.c
> >> >>>> @@ -85,7 +85,7 @@ static inline void close_pair(int fd[2])
> >> >>>>   }
> >> >>>>
> >> >>>>   #ifndef GIT_WINDOWS_NATIVE
> >> >>>> -static inline void dup_devnull(int to)
> >> >>>> +void dup_devnull(int to)
> >> >>>>   {
> >> >>>
> >> >>>
> >> >>> The #ifndef GIT_WINDOWS_NATIVE rings very, very loud alarm bells.
> >> >>
> >> >>
> >> >> Yeah, but I must say that I don't know what I should do about this.
> >> >> Do you have a suggestion? Should I try to implement the same function
> >> >> for Windows?
> >
> > No, you should change the code that requires that ugly dup()ing so that it
> > can be configured to shut up.
> 
> After taking a look, it looks like a routine that does nothing could
> be passed to set_error_routine() and that could do part of the trick.
> 
> This part might not be too ugly, but it would anyway be more complex,
> less close to what the code is doing now and more error prone, as one
> also need to make sure that for example no warning() or
> fprintf(stderr, ...) are called and nothing is printed on stdout.

I am afraid that you *have* to do that, though, if you truly want to
libify the code.

Of course you can go with really ugly workarounds instead. Something like
a global flag that die() and error() and warning() respect. It would
incur some technical debt, but it would make your life easier in the short
run.

Both the real solution and the workaround would be better than the current
version of the patches that dup() back and forth, just to avoid addressing
the real problem.

Ciao,
Dscho