A question on wait_reading_process_output

A question on wait_reading_process_output

[Herbert Euler]

I have a question on this function, more precisely, line 4460 through
4496 in process.c.  The following predicates shows how that code would
be entered:

      if (wait_proc
	  && ! EQ (wait_proc->status, Qrun)
	  && ! EQ (wait_proc->status, Qconnect))

That is, when wait_proc is not running anymore.  The output from
wait_proc is read in the body of the if, and after that, the last
statement, break, causes the execution quiting from the outmost loop.
The value of got_some_input becomes the value of the calling to
wait_reading_process_output after the quiting.

There seems a problem here: The value of got_some_input isn't set to
nonzero during the reading happens inside the body of the if even when
something has been read, so it is possible that something has been
read from wait_proc's output but got_some_input is still zero.  In
that case, the return value of wait_reading_process is not what it
promises.

It's hard to produce such a case to reveal this case, so it remains my
guess.  Perhaps I missed something, but is my guess right?  Thanks in
advance.

Regards,
Guanpeng Xu
_________________________________________________________________
Invite your mail contacts to join your friends list with Windows Live Spaces. It's easy!
http://spaces.live.com/spacesapi.aspx?wx_action=create&wx_url=/friends.aspx&mkt=en-us

Re: A question on wait_reading_process_output

[Stefan Monnier]

> There seems a problem here: The value of got_some_input isn't set to
> nonzero during the reading happens inside the body of the if even when
> something has been read, so it is possible that something has been
> read from wait_proc's output but got_some_input is still zero.  In
> that case, the return value of wait_reading_process is not what it
> promises.

So you're proposing a patch like the onw below?
I guess that'd make sense.  Does anybody see a problem with it?


        Stefan


--- process.c.~1.542.~	2008-05-22 17:12:27.000000000 -0400
+++ process.c	2008-05-29 10:55:25.000000000 -0400
@@ -4475,7 +4479,10 @@
 		break;
 
               if (0 < nread)
+                {
                 total_nread += nread;
+		  got_some_input = 1;
+		}
 #ifdef EIO
 	      else if (nread == -1 && EIO == errno)
                 break;

RE: A question on wait_reading_process_output

[Herbert Euler]

>> There seems a problem here: The value of got_some_input isn't set to
>> nonzero during the reading happens inside the body of the if even when
>> something has been read, so it is possible that something has been
>> read from wait_proc's output but got_some_input is still zero.  In
>> that case, the return value of wait_reading_process is not what it
>> promises.
>
> So you're proposing a patch like the onw below?

Based on the fact I see, yes.  In fact I prefer changing
got_some_input outside the inner reading loop, like this:

Regards,
Guanpeng Xu

Index: process.c
===================================================================
RCS file: /sources/emacs/emacs/src/process.c,v
retrieving revision 1.542
diff -c -F '^[_a-zA-Z0-9$]+ *(' -r1.542 process.c
*** process.c   22 May 2008 14:53:26 -0000      1.542
--- process.c   30 May 2008 01:24:24 -0000
*************** wait_reading_process_output (time_limit,
*** 4489,4494 ****
--- 4489,4495 ----
                  break;
  #endif
            }
+           got_some_input = total_nread> 0 ? 1 : 0;
          if (total_nread> 0 && do_display)
            redisplay_preserve_echo_area (10);
_________________________________________________________________
Invite your mail contacts to join your friends list with Windows Live Spaces. It's easy!
http://spaces.live.com/spacesapi.aspx?wx_action=create&wx_url=/friends.aspx&mkt=en-us

Re: A question on wait_reading_process_output

[Stefan Monnier]

> +           got_some_input = total_nread> 0 ? 1 : 0;

Which, by the η-reduction rule, can be replaced advantageously by:

          got_some_input = total_nread > 0;


– Stefan

RE: A question on wait_reading_process_output

[Herbert Euler]

>> +           got_some_input = total_nread> 0 ? 1 : 0;
> 
> Which, by the η-reduction rule, can be replaced advantageously by:
> 
>           got_some_input = total_nread> 0;

Thanks, it is surely better  :)

Regards,
Guanpeng Xu
_________________________________________________________________
News, entertainment and everything you care about at Live.com. Get it now!
http://www.live.com/getstarted.aspx