GNU bug report logs - #64975
30.0.50; accept-process-output and async connect

Previous Next

Package: emacs;

Reported by: Helmut Eller <eller.helmut <at> gmail.com>

Date: Mon, 31 Jul 2023 13:32:01 UTC

Severity: normal

Found in version 30.0.50

Full log


Message #23 received at 64975 <at> debbugs.gnu.org (full text, mbox):

From: Robert Pluim <rpluim <at> gmail.com>
To: Helmut Eller <eller.helmut <at> gmail.com>
Cc: 64975 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#64975: 30.0.50; accept-process-output and async connect
Date: Tue, 08 Aug 2023 17:16:52 +0200
>>>>> On Tue, 08 Aug 2023 16:36:00 +0200, Helmut Eller <eller.helmut <at> gmail.com> said:

    Helmut> On Tue, Aug 08 2023, Robert Pluim wrote:
    >> I think itʼs correct, as I have a change locally setting
    >> got_some_output for a different test case, but Iʼm going to be a pain,
    >> and ask Helmut to explain why, and see if I agree with his explanation
    >> (thatʼs a very hairy loop)

    Helmut> Setting got_some_output=1, was the first thing that came to mind that
    Helmut> made the test case pass :-).  A reasonable strategy, if we have a
    Helmut> comprehensive test suite.  If the test suite is lacking, then writing
    Helmut> more tests is a good investment too.  Ahem.

Submitting new tests is always good :-)

    Helmut> Setting got_some_output=1 will terminate the while(1) loop, but only on
    Helmut> the next iteration (around line process.c:5753) and only after another
    Helmut> useless call to xg_select.  So maybe a change like below might be
    Helmut> better.

It also means we finish looping through all the channels, unlike your
patch below. I think thatʼs a smaller and thus better change, and
aligns more with the docstring:

    Allow any pending output from subprocesses to be read by Emacs.
    It is given to their filter functions.

So if the rest of the test cases pass, I think we should apply your
original patch.

    Helmut> The variable got_some_output is also the return value of
    Helmut> wait_reading_process_output.  So I thought that 1 is a reasonable value
    Helmut> to indicate "some event happened".  0 and negative values are converted
    Helmut> to nil in accept-process-output, so there isn't an obvious way to
    Helmut> indicate "not a timeout, 0 bytes read, but some other event".  Maybe
    Helmut> MAX_INT could be used.

1 is ok as a value.

    Helmut> If you're asking why accept-process-output should return at all, then
    Helmut> the answer is that the socket is now writable and the caller probably
    Helmut> want's to know that.

I think thatʼs ok, since any actual input received from the process
will get passed to the filter function anyway.

Eli, the docstring also says

    Optional argument PROCESS means to return only after output is
    received from PROCESS or PROCESS closes the connection.

Do we need to add something like "or the underlying network connection
becomes available"? (I wonder if thatʼs too strong a guarantee).

Thanks

Robert
-- 




This bug report was last modified 1 year and 312 days ago.

Previous Next


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