Ludovic Courtès <ludo@gnu.org> writes:
Toggle quote (12 lines)
> Hi Christopher,
>
> Christopher Baines <mail@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.
Toggle quote (23 lines)
>> +++ b/guix/inferior.scm
>> @@ -156,12 +156,14 @@ (define (open-bidirectional-pipe command . args)
>> (close-port parent)
>> (close-fdes 0)
>> (close-fdes 1)
>> + (close-fdes 2)
>> (dup2 (fileno child) 0)
>> (dup2 (fileno child) 1)
>> ;; Mimic 'open-pipe*'.
>> - (unless (file-port? (current-error-port))
>> - (close-fdes 2)
>> - (dup2 (open-fdes "/dev/null" O_WRONLY) 2))
>> + (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))
Toggle quote (3 lines)
> 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.
Thanks,
Chris
-----BEGIN PGP SIGNATURE-----
iQKlBAEBCgCPFiEEPonu50WOcg2XVOCyXiijOwuE9XcFAmK5mqNfFIAAAAAALgAo
aXNzdWVyLWZwckBub3RhdGlvbnMub3BlbnBncC5maWZ0aGhvcnNlbWFuLm5ldDNF
ODlFRUU3NDU4RTcyMEQ5NzU0RTBCMjVFMjhBMzNCMEI4NEY1NzcRHG1haWxAY2Jh
aW5lcy5uZXQACgkQXiijOwuE9XchPQ//YAAcZAUwEHAHb8NWWU91G1cr1xgacXfP
mLwyCr3kREI8yd3JBfuj84bR2NRMz5ddvxC/pveR7hh/91hEvpR7n0RTkFXO9P7i
B15qLNJ87QDraX4AS0qMvhY7adbNRD7U7fzK4jXY7uqUln4SEdd9IppW+EivIipy
3YQVbSNl6YCWT2Ap+LPIowXl5r3WHhVGSQKApqIC2IqHmmgW/4VTiO3GClRqlIBl
nrG4buaQMCj/EOaki03bckqh6KPSeoGwCyRDCcqPjrfBDg3nOhOHZSsBt0Mooktq
+mc2Icy6Ibmibu9c2hckwioc17v2TvuHUcOWYD6w9Mbhmmx4fc9EbxXHMxx7wEnK
MeWaWwYQwGvYT1KRMbEa8XbFJjBf0Ugyogu1c2WJohYds2n8yYqmbr10dJhCw/m7
rRcB0CI7MkeQgk2lVzMKFb4+6GmwY4PLCTOju7UcszsWwE2ubZ9BGLScUSstBZl1
78aM5giS3M7TrwIJj81rMeIvVPCdiFuF0fNfwdJWcS1pZQLyE0akfyvqqtzyKJTM
sf1IWetVbErUcBfzGZEdkqmBOU6aSSr6esed+CuQDxTDCH5CxO4Q984p/DT83jVA
H7dsOViglcMuE9B4h3Gl57+DAp/iRFZusQrfsVJtX75HdHjioP7nV3hnhrRq4BU0
ijd1psGfW3M=
=x3+O
-----END PGP SIGNATURE-----