Re: wait_reading_process_ouput hangs in certain cases (w/ patches)

[Matthias Dahl <ml_emacs-lists@binary-island.eu>]

Hello Paul,

Thanks for your feedback!

On 12/11/17 22:17, Paul Eggert wrote:

> This will include the carryover in the number of bytes read, even though
> this code did not read the carryover bytes. Is that what you intended?

Good catch -- not what I intended. Sorry for missing that. I will update
the patches once the rest of your suggestions are discussed / decided.

> This is overkill, as the total amount of bytes read by a call to
> read_process_output cannot exceed 4096, so all we need is an unsigned
> counter with more than 12 bits. How about making it 'unsigned int'
> instead? It could even be 'unsigned short', though that might be
> overkill. Whatever size is chosen, the comment should say that the value
> recorded is the true value modulo the word size.

That would not be enough. The counter is for the entire process lifetime
and not just for a single read back or chain of recursive read backs.
That is why it is EMACS_UINT and not just a smallish data type.

What you suggest we discussed shortly earlier in this thread but decided
against it and go for this simpler solution.

>   if (wait_proc)
>     {
>       unsigned int diff = (wait_proc->infd_num_bytes_read
>                            - initial_wait_proc_num_bytes_read);
>       if (diff != 0)
>         got_some_output = diff;
>     }
>
> which is still a bit simpler than what was proposed. Anyway there's no
> need to refer to ISO/IEC 9899:1999 chapter and verse here, any more
> than there's a need to refer to it in the countless other places that
> we rely on it.

Personally I think the current version is a bit more clear and easier
to understand from a semantic point of view. But that might just be me.
So I would rather go with the current version. Generally, one can argue
for both.

Regarding the ISO/IEC commentary, I thought it was worth mentioning here
since it is an important point to make that not everybody might know.

But if there is consensus, I will remove the commentary and update the
if-statement to your version. Eli, what are your thoughts?

As soon as we have finalized those points, I will send updated patches.
And thanks again for catching the bug!

Have a nice day,
Matthias

-- 
Dipl.-Inf. (FH) Matthias Dahl | Software Engineer | binary-island.eu