unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
From: Paul Eggert <eggert@cs.ucla.edu>
To: Matthias Dahl <ml_emacs-lists@binary-island.eu>,
	Eli Zaretskii <eliz@gnu.org>
Cc: emacs-devel@gnu.org
Subject: Re: wait_reading_process_ouput hangs in certain cases (w/ patches)
Date: Sun, 12 Nov 2017 13:17:33 -0800	[thread overview]
Message-ID: <ea90aaf0-263f-f032-2549-c147c2e595e8@cs.ucla.edu> (raw)
In-Reply-To: <398f8d17-b727-d5d6-4a31-772448c7ca0d@binary-island.eu>

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.



  parent reply	other threads:[~2017-11-12 21:17 UTC|newest]

Thread overview: 151+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-24 18:52 wait_reading_process_ouput hangs in certain cases (w/ patches) Matthias Dahl
2017-10-25 14:53 ` Eli Zaretskii
2017-10-26 14:07   ` Matthias Dahl
2017-10-26 16:23     ` Eli Zaretskii
2017-10-26 18:56       ` Matthias Dahl
2017-10-28  8:20         ` Matthias Dahl
2017-10-28  9:28         ` Eli Zaretskii
2017-10-30  9:48           ` Matthias Dahl
2017-11-03  8:52             ` Matthias Dahl
2017-11-03  9:58               ` Eli Zaretskii
2017-11-04 12:11             ` Eli Zaretskii
2017-11-06 14:15               ` Matthias Dahl
2017-11-06 16:34                 ` Eli Zaretskii
2017-11-06 18:24                   ` Paul Eggert
2017-11-06 20:17                     ` Eli Zaretskii
2017-11-07 14:18                   ` Matthias Dahl
2017-11-07 16:40                     ` Eli Zaretskii
2017-11-10 14:45                       ` Matthias Dahl
2017-11-10 15:25                         ` Eli Zaretskii
2017-11-12 21:17                         ` Paul Eggert [this message]
2017-11-13  3:27                           ` Eli Zaretskii
2017-11-13  5:27                             ` Paul Eggert
2017-11-13 16:00                               ` Eli Zaretskii
2017-11-13 19:42                                 ` Paul Eggert
2017-11-13 20:12                                   ` Eli Zaretskii
2017-11-13 14:13                           ` Matthias Dahl
2017-11-13 16:10                             ` Eli Zaretskii
2017-11-14 15:05                               ` Matthias Dahl
2017-11-13 19:44                             ` Paul Eggert
2017-11-14 14:58                               ` Matthias Dahl
2017-11-14 15:24                                 ` Paul Eggert
2017-11-14 16:03                                   ` Eli Zaretskii
2017-11-14 16:23                                     ` Eli Zaretskii
2017-11-14 21:54                                       ` Paul Eggert
2017-11-15 14:03                                         ` Matthias Dahl
2017-11-16 15:37                                           ` Eli Zaretskii
2017-11-16 16:46                                           ` Paul Eggert
2017-11-18 14:24                                             ` Matthias Dahl
2017-11-18 14:51                                               ` Eli Zaretskii
2017-11-18 17:14                                                 ` Stefan Monnier
2017-11-19  7:07                                               ` Paul Eggert
2017-11-19 15:42                                                 ` Eli Zaretskii
2017-11-19 17:06                                                   ` Paul Eggert
2017-11-20 15:29                                                 ` Matthias Dahl
2017-11-21 14:44                                                   ` Matthias Dahl
2017-11-21 21:31                                                     ` Clément Pit-Claudel
2017-11-22 14:14                                                       ` Matthias Dahl
2017-11-22  8:55                                                     ` Paul Eggert
2017-11-22 14:33                                                       ` Matthias Dahl
2017-11-24  2:31                                                         ` Stefan Monnier
2017-12-28 17:52                                                         ` Eli Zaretskii
2017-12-04  9:40                                                       ` Matthias Dahl
2018-02-13 14:25                                                         ` Matthias Dahl
2018-02-13 16:56                                                           ` Paul Eggert
2018-02-16 16:01                                                           ` Eli Zaretskii
2018-02-16 16:09                                                             ` Lars Ingebrigtsen
2018-02-16 16:54                                                               ` Lars Ingebrigtsen
2018-02-22 11:45                                                               ` andres.ramirez
2018-02-26 14:39                                                                 ` Matthias Dahl
2018-02-26 15:11                                                                   ` andrés ramírez
2018-02-26 15:17                                                                     ` Lars Ingebrigtsen
2018-02-26 15:29                                                                       ` andrés ramírez
2018-02-26 16:52                                                                         ` Daniel Colascione
2018-02-26 17:19                                                                           ` andrés ramírez
2018-02-26 17:24                                                                             ` Daniel Colascione
2018-02-27  1:53                                                                               ` Re: andrés ramírez
2018-02-27  9:15                                                                       ` wait_reading_process_ouput hangs in certain cases (w/ patches) Matthias Dahl
2018-02-27 12:01                                                                         ` Lars Ingebrigtsen
2018-02-27  9:11                                                                     ` Matthias Dahl
2018-02-27 11:54                                                                       ` andrés ramírez
2018-02-27 15:02                                                                         ` Matthias Dahl
2018-02-27 15:13                                                                           ` Lars Ingebrigtsen
2018-02-27 15:17                                                                             ` Matthias Dahl
2018-02-27 15:19                                                                               ` Lars Ingebrigtsen
2018-02-27 15:14                                                                         ` Matthias Dahl
2018-02-27 15:17                                                                           ` Lars Ingebrigtsen
2018-03-01 10:42                                                                           ` Lars Ingebrigtsen
2018-03-01 14:36                                                                             ` Matthias Dahl
2018-03-01 15:10                                                                               ` andrés ramírez
2018-03-01 16:30                                                                                 ` T.V Raman
2018-03-01 16:46                                                                                   ` andrés ramírez
2018-03-01 18:23                                                                                     ` T.V Raman
2018-03-01 19:13                                                                                     ` Eli Zaretskii
2018-03-02 20:21                                                                                       ` andrés ramírez
2018-03-03  7:55                                                                                         ` Eli Zaretskii
2018-03-05 14:43                                                                             ` Matthias Dahl
2018-03-05 14:44                                                                               ` Lars Ingebrigtsen
2018-03-05 14:54                                                                                 ` Matthias Dahl
2018-03-13  9:54                                                                                 ` Matthias Dahl
2018-03-13 12:35                                                                                   ` Robert Pluim
2018-03-13 13:40                                                                                     ` Robert Pluim
2018-03-13 15:10                                                                                     ` Matthias Dahl
2018-03-13 15:30                                                                                       ` Robert Pluim
2018-03-13 15:36                                                                                       ` Dmitry Gutov
2018-03-13 15:46                                                                                         ` Robert Pluim
2018-03-13 15:56                                                                                           ` Dmitry Gutov
2018-03-13 16:57                                                                                             ` Robert Pluim
2018-03-13 18:03                                                                                               ` Eli Zaretskii
2018-03-13 20:12                                                                                                 ` Robert Pluim
2018-03-14 14:21                                                                                         ` Matthias Dahl
2018-03-13 16:32                                                                                       ` Lars Ingebrigtsen
2018-03-14  9:32                                                                                         ` Matthias Dahl
2018-03-14 14:55                                                                                           ` Lars Ingebrigtsen
2018-03-31 15:44                                                                                       ` Lars Ingebrigtsen
2018-04-01  2:05                                                                                         ` andrés ramírez
2018-06-08  5:11                                                                                         ` Leo Liu
2018-06-08  6:57                                                                                           ` Eli Zaretskii
2018-06-08  9:07                                                                                             ` Leo Liu
2018-03-13 16:12                                                                                   ` Eli Zaretskii
2018-03-14  4:16                                                                                     ` Leo Liu
2018-03-14  9:22                                                                                       ` Robert Pluim
2018-03-15  0:37                                                                                         ` Leo Liu
2018-03-14 15:09                                                                                       ` andrés ramírez
2018-03-14 16:45                                                                                       ` Eli Zaretskii
2018-03-15  1:03                                                                                         ` Leo Liu
2018-03-15  7:55                                                                                           ` Robert Pluim
2018-03-14 22:54                                                                                       ` Stefan Monnier
2018-03-15  1:06                                                                                         ` Leo Liu
2018-03-14  9:56                                                                                     ` Matthias Dahl
2018-03-14 12:24                                                                                       ` Stefan Monnier
2018-03-14 14:34                                                                                         ` Matthias Dahl
2018-03-14 22:52                                                                                           ` Stefan Monnier
2018-03-15 15:17                                                                                             ` Matthias Dahl
2018-03-14 16:43                                                                                       ` Eli Zaretskii
2018-03-15 14:59                                                                                         ` Matthias Dahl
2018-06-26 13:36                                                                                           ` Matthias Dahl
2018-06-26 14:09                                                                                             ` andrés ramírez
2018-06-27 13:10                                                                                               ` Matthias Dahl
2018-06-27 15:18                                                                                                 ` Eli Zaretskii
2018-06-28  8:01                                                                                                   ` Matthias Dahl
2018-06-28 13:04                                                                                                     ` Eli Zaretskii
2018-06-28 13:25                                                                                                       ` Matthias Dahl
2018-06-28 16:33                                                                                                         ` Leo Liu
2018-06-28 18:31                                                                                                           ` T.V Raman
2018-07-02 13:27                                                                                                             ` Matthias Dahl
2018-07-02 14:35                                                                                                               ` T.V Raman
2018-07-03 13:27                                                                                                                 ` Matthias Dahl
2018-07-03 13:52                                                                                                                   ` T. V. Raman
2018-07-03 15:03                                                                                                                     ` Stefan Monnier
2018-07-02 13:24                                                                                                           ` Matthias Dahl
2018-07-14  2:27                                                                                                             ` Leo Liu
2018-07-03 13:34                                                                                                       ` Matthias Dahl
2018-07-03 18:57                                                                                                         ` Eli Zaretskii
2018-07-04  7:35                                                                                                           ` Matthias Dahl
2018-07-04 15:11                                                                                                             ` Eli Zaretskii
2018-07-21  9:52                                                                                             ` Eli Zaretskii
2018-08-07  8:38                                                                                               ` Matthias Dahl
2018-08-07 17:10                                                                                                 ` Paul Eggert
2018-09-10  8:21                                                                                                 ` Eli Zaretskii
2017-11-07 17:23                     ` Stefan Monnier
2017-11-10 14:53                       ` Matthias Dahl

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ea90aaf0-263f-f032-2549-c147c2e595e8@cs.ucla.edu \
    --to=eggert@cs.ucla.edu \
    --cc=eliz@gnu.org \
    --cc=emacs-devel@gnu.org \
    --cc=ml_emacs-lists@binary-island.eu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).