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

[Paul Eggert]

Matthias Dahl wrote:

>    /* Now set NBYTES how many bytes we must decode.  */
>    nbytes += carryover;
>  
> +  p->infd_num_bytes_read += nbytes;

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?

> +    /* Byte-count for process output read from `infd'.  */
> +    EMACS_UINT infd_num_bytes_read;

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.

> +          /* Timers could have called `accept-process-output', thus reading 
> the output
> +             of wait_proc while we (in the worst case) wait endlessly for it 
> to become
> +             available later. So we need to check if data has been read and 
> break out
> +             early if that is so since our job has been fulfilled. */
> +          if (wait_proc
> +              && wait_proc->infd_num_bytes_read != 
> initial_wait_proc_num_bytes_read)
> +            {
> +              /* Computations on unsigned types are well defined and won't 
> overflow,
> +                 so this is safe even if our initial value > our current 
> value, in
> +                 case of a wrap around. (ISO/IEC 9899:1999 §6.2.5/9) */
> +              got_some_output = wait_proc->infd_num_bytes_read
> +                                - initial_wait_proc_num_bytes_read;
> +            }
> +
All that matters for got_some_output is whether it is negative, zero, or 
positive. So I suggest replacing the above with the following, as it's a bit 
faster and simpler and doesn't require commentary:

> +          if (wait_proc
> +              && wait_proc->infd_num_bytes_read != 
> initial_wait_proc_num_bytes_read)
> +            got_some_output = 1;

Similarly for the other change that assigns to got_some_output.