From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Matthias Dahl Newsgroups: gmane.emacs.devel Subject: Re: wait_reading_process_ouput hangs in certain cases (w/ patches) Date: Mon, 13 Nov 2017 15:13:28 +0100 Message-ID: References: <83lgjz8eiy.fsf@gnu.org> <831slp98ut.fsf@gnu.org> <83tvyj62qg.fsf@gnu.org> <83r2tetf90.fsf@gnu.org> <5150d198-8dd3-9cf4-5914-b7e945294452@binary-island.eu> <83tvy7s6wi.fsf@gnu.org> <83inemrqid.fsf@gnu.org> <398f8d17-b727-d5d6-4a31-772448c7ca0d@binary-island.eu> NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Trace: blaine.gmane.org 1510582513 6021 195.159.176.226 (13 Nov 2017 14:15:13 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Mon, 13 Nov 2017 14:15:13 +0000 (UTC) User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 Cc: emacs-devel@gnu.org To: Paul Eggert , Eli Zaretskii Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Mon Nov 13 15:15:05 2017 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by blaine.gmane.org with esmtp (Exim 4.84_2) (envelope-from ) id 1eEFVw-0001CI-VW for ged-emacs-devel@m.gmane.org; Mon, 13 Nov 2017 15:15:05 +0100 Original-Received: from localhost ([::1]:54625 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eEFW4-00073H-4M for ged-emacs-devel@m.gmane.org; Mon, 13 Nov 2017 09:15:12 -0500 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:48691) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eEFUY-0006ka-3B for emacs-devel@gnu.org; Mon, 13 Nov 2017 09:13:42 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1eEFUS-0000Uj-DZ for emacs-devel@gnu.org; Mon, 13 Nov 2017 09:13:38 -0500 Original-Received: from ud19.udmedia.de ([194.117.254.59]:43074 helo=mail.ud19.udmedia.de) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1eEFUS-0000TA-4B for emacs-devel@gnu.org; Mon, 13 Nov 2017 09:13:32 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=simple; d=binary-island.eu; h= subject:to:cc:references:from:message-id:date:mime-version :in-reply-to:content-type:content-transfer-encoding; s=k1; bh=JH E2Ro6R0yZIjuwfNHjkvWwNFAvvh8EPa2XJGD5Pre8=; b=OrEya3XQ+9ldaNmPgQ gw3hoDIAsYN1oIVVGBlOgbzYm2m/l7Ex2gTKmwmjouO8uyq9UMwYLJQXJntV5gCl AwNP3GsPO5CzPxqNp9lUGSyVESy2Xx9nHIZq6YmtV4JKdvFE2vKbg/r1niwrDZhA AXkFYmDc/Cc3keEQRw4i+QJdc= Original-Received: (qmail 21159 invoked from network); 13 Nov 2017 15:13:27 +0100 Original-Received: from unknown (HELO ?IPv6:2a02:810b:c540:234:36aa:25b9:ca8f:d05f?) (ud19?126p1@2a02:810b:c540:234:36aa:25b9:ca8f:d05f) by mail.ud19.udmedia.de with ESMTPSA (ECDHE-RSA-AES128-GCM-SHA256 encrypted, authenticated); 13 Nov 2017 15:13:27 +0100 Openpgp: id=1E87ADA02EFE759EFC20B2D1042F47D273AA780C In-Reply-To: Content-Language: en-US X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 194.117.254.59 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.org gmane.emacs.devel:220138 Archived-At: 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