GNU bug report logs - #56218
[PATCH] guix: inferior: Fix the behaviour of open-inferior #:error-port.

Previous Next

Full log

View this message in rfc822 format

From: Ludovic Courtès <ludo <at> gnu.org>
To: Christopher Baines <mail <at> cbaines.net>
Cc: 56218 <at> debbugs.gnu.org
Subject: [bug#56218] [PATCH] guix: inferior: Fix the behaviour of open-inferior #:error-port.
Date: Tue, 05 Jul 2022 17:12:58 +0200

Hi,

Christopher Baines <mail <at> cbaines.net> skribis:

> Ludovic Courtès <ludo <at> gnu.org> writes:
>
>> Hi Christopher,
>>
>> Christopher Baines <mail <at> cbaines.net> skribis:
>>
>>> This should be the error port used by the inferior process, but currently it's
>>> either stderr if #:error-port is a file port, or /dev/null otherwise.
>>
>> That’s still the case with this patch, no?
>>
>> The patch does make a difference when (current-error-port) wraps a file
>> descriptor other than 2 though.
>
> Maybe this sentance is a little unclear.
>
> What I'm trying to say is that passing a port as #:error-port doesn't
> really work. There's no scenario where the output actually goes to the
> port you provide, though it can have some effect.

OK, I think I got it.

>>> +            (dup2 (if (file-port? (current-error-port))
>>> +                      (fileno (current-error-port))
>>> +                      (open-fdes "/dev/null" O_WRONLY))
>>> +                  2)
>>
>> If (current-error-port) wraps FD 2 when the function is called, then, by
>> the time we reach (dup2 … 2), the FD behind (current-error-port) has be
>> closed; we end up doing (dup2 2 2), but FD 2 is closed, so we get EBADF.
>>
>> Or am I misunderstanding?
>
> That sounds reasonable, I've only tested this change in the scenario
> when the #:error-port isn't stderr, and I mostly adapted this from what
> I thought open-pipe* did.
>
> Maxime suggested using move->fdes, so maybe this would be an improved
> version:
>
>   ;; Mimic 'open-pipe*'.
>   (if (file-port? (current-error-port))
>       (unless (eq? (fileno (current-error-port)) 2)
>         (move-fdes (current-error-port) 2))
>       (move->fdes (open-file "/dev/null" O_WRONLY) 2))

I prefer the original version: I find it clearer (it’s low-level) and
probably more robust (thinking through the port/FD interaction needs is
more demanding :-)).

>> Perhaps we should add one test for each case (error port is a file port
>> vs. error port is another kind of port) in ‘tests/inferior.scm’.
>
> Yep, sounds good.

To sum up: I think it’s a welcome change, and it’s even more welcome
with a couple of tests to make sure it behaves the way we think it does.

Thanks!

Ludo’.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.