QNX subprocess bug

QNX subprocess bug

[Bob Bramwell]

I submitted an article about this a month or so ago but never saw it come 
around, so my apologies if this is old news to you.

The bug is showing up in emacs 21.2.1 on QNX 6.3.0.  It appears that process IDs 
  on QNX really occupy 32-bits.  Some of them are quite large.  This causes a 
problem in the subprocess package which uses an elisp integer (28 bits + type 
info - I think) to try to store the PID.  An added complication is that there is 
a cheap-and-cheerful "just stuff this value into this elisp int" macro which (in 
this case) has the effect of clobbering the type bits, ultimately resulting in 
what appears to be a garbage collector infinite recursion crash.

Obvious this problem could, in principle, arise on any system that uses 32-bit 
process IDs, and it seems to me rather sloppy programming to use such a 
shortcut, especially in such comparatively rarely executed code.

It looks as though there ought to be some way to rewrite the code with the 
integer represented as a kind of indirect object, but I don't feel qualified to 
do this without consulting The Experts.  Anyone wish to provide some guidance? 
I will gladly fix the problem given a few pointers.

Re: QNX subprocess bug

[Richard Stallman]

    The bug is showing up in emacs 21.2.1 on QNX 6.3.0.  It appears
    that process IDs on QNX really occupy 32-bits.  Some of them are
    quite large.  This causes a problem in the subprocess package
    which uses an elisp integer (28 bits + type info - I think) to
    try to store the PID.

    Obvious this problem could, in principle, arise on any system that uses 32-bit 
    process IDs, and it seems to me rather sloppy programming to use such a 
    shortcut, especially in such comparatively rarely executed code.

It is not a short cut.  The reason the pid is represented as a Lisp
integer is that (1) all the slots in a process object have to be Lisp
objects, and (2) Emacs has no way to deal with integer values larger
than a Lisp integer.

It will not be easy to fix this problem.  Since QNX support is not a
high priority for GNU, I think it is up to you to implement the fix.
But since there is no obvious way to fix it, first you'd need to
work out a way that would not cause too much problem in Emacs.

Re: QNX subprocess bug

[Eli Zaretskii]

> From: Richard Stallman <rms@gnu.org>
> Date: Sat, 25 Mar 2006 19:22:02 -0500
> Cc: emacs-devel@gnu.org
> 
> It is not a short cut.  The reason the pid is represented as a Lisp
> integer is that (1) all the slots in a process object have to be Lisp
> objects, and (2) Emacs has no way to deal with integer values larger
> than a Lisp integer.
> 
> It will not be easy to fix this problem.  Since QNX support is not a
> high priority for GNU, I think it is up to you to implement the fix.
> But since there is no obvious way to fix it, first you'd need to
> work out a way that would not cause too much problem in Emacs.

Maybe the easiest way to fix this is to use a Lisp float to record the
PID.

Re: QNX subprocess bug

[Richard Stallman]

    Maybe the easiest way to fix this is to use a Lisp float to record the
    PID.

I would not mind if that were done on QNX, _provided_ it is not a pain
in the neck for maintenance on other systems.

    > (1) all the slots in a process object have to be Lisp objects,

    Actually, this doesn't have to be true.  Several other kinds of similar
    (vectorlike) objects have lifted this limitation, e.g. buffers.

Yes, but that means we have to do more work in GC.

    Actually, Emacs can store such values in floating point numbers, or encode
    them in cons-cells (as is done for time values as well as file sizes).

Using cons cells to represent pids seems likely to cause trouble in
any code that uses pids.

Re: QNX subprocess bug

[Kim F. Storm]

Richard Stallman <rms@gnu.org> writes:

> Using cons cells to represent pids seems likely to cause trouble in
> any code that uses pids.

I don't know if it is true, but someone recently told me that even the
Linux kernel will use 32-bit pids if you happen to create more than
32000 processes at the same time.

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: QNX subprocess bug

[Richard Stallman]

    I don't know if it is true, but someone recently told me that even the
    Linux kernel will use 32-bit pids if you happen to create more than
    32000 processes at the same time.

Does that really mean all 32 bits get used?

There are not a gigantic number of places in the code that call
process-id.  But at least one prints the pid in decimal, so using a
cons cell would not work well.

Using a float would work.

Re: QNX subprocess bug

[Stefan Monnier]

>     The bug is showing up in emacs 21.2.1 on QNX 6.3.0.  It appears
>     that process IDs on QNX really occupy 32-bits.  Some of them are
>     quite large.  This causes a problem in the subprocess package
>     which uses an elisp integer (28 bits + type info - I think) to
>     try to store the PID.

>     Obvious this problem could, in principle, arise on any system that
>     uses 32-bit  process IDs, and it seems to me rather sloppy programming
>     to use such a  shortcut, especially in such comparatively rarely
>     executed code.

> It is not a short cut.  The reason the pid is represented as a Lisp
> integer is that
> (1) all the slots in a process object have to be Lisp objects,

Actually, this doesn't have to be true.  Several other kinds of similar
(vectorlike) objects have lifted this limitation, e.g. buffers.
We could easily lift this limitation.
That would also allow us to replace raw_status_(high|low) with just
raw_status.

> and (2) Emacs has no way to deal with integer values larger
> than a Lisp integer.

Actually, Emacs can store such values in floating point numbers, or encode
them in cons-cells (as is done for time values as well as file sizes).


        Stefan

Re: QNX subprocess bug

[bob]

I have observed PIDs very high in the 32-bit space (most user processes) and 
quite low (most system processes) and very little in between.  Right now I don't 
have access to my QNX system so I can't give you any real examples.  QNX is not 
open source so there is no obvious way to tell how the PIDs get generated.

The float (I take it we're talking "double" here) sounds like the best solution. 
I'll hack it into the code and see what happens.

Thanks!

>Date: Thu, 30 Mar 2006 22:11:02 -0500
>From: Richard Stallman <rms@gnu.org>
>Subject: Re: QNX subprocess bug
>To: storm@cua.dk (Kim F. Storm)
>Cc: bbramwel@shaw.ca, emacs-devel@gnu.org
>X-BadHeader: plain; charset=ISO-8859-15
>Original-recipient: rfc822;bbramwel@shaw.ca
>
>    I don't know if it is true, but someone recently told me that even the
>    Linux kernel will use 32-bit pids if you happen to create more than
>    32000 processes at the same time.
>
>Does that really mean all 32 bits get used?
>
>There are not a gigantic number of places in the code that call
>process-id.  But at least one prints the pid in decimal, so using a
>cons cell would not work well.
>
>Using a float would work.

                 | 
Bob Bramwell     | Many people would sooner die than think; 
ProntoLogical    | in fact, they do so.
+1 403/861-8827  |  - Bertrand Russell
                 | 

Re: QNX subprocess bug

[Stefan Monnier]

> I have observed PIDs very high in the 32-bit space (most user processes)
> and  quite low (most system processes) and very little in between.
> Right now I don't  have access to my QNX system so I can't give you any
> real examples.  QNX is not open source so there is no obvious way to tell
> how the PIDs get generated.

> The float (I take it we're talking "double" here) sounds like the
> best solution.  I'll hack it into the code and see what happens.

I think the best solution is to keep pid_t values (without changing them
into Lisp_Object values) as long as possible, and to use floats when turning
them into Lisp_Object.

The patch below does just that.  It has been lightly tested.


        Stefan


* looking for monnier@iro.umontreal.ca--first/emacs--monnier--0--patch-347 to compare with
* comparing to monnier@iro.umontreal.ca--first/emacs--monnier--0--patch-347
M  src/process.c
M  src/alloc.c
M  src/lisp.h
M  src/process.h

* modified files

--- orig/src/process.h
+++ mod/src/process.h
@@ -51,8 +51,6 @@
     Lisp_Object log;
     /* Buffer that output is going to */
     Lisp_Object buffer;
-    /* Number of this process */
-    Lisp_Object pid;
     /* t if this is a real child process.
        For a net connection, it is a plist based on the arguments to make-network-process.  */
     Lisp_Object childp;
@@ -63,10 +61,6 @@
     /* Non-nil means kill silently if Emacs is exited.
        This is the inverse of the `query-on-exit' flag.  */
     Lisp_Object kill_without_query;
-    /* Record the process status in the raw form in which it comes from `wait'.
-       This is to avoid consing in a signal handler.  */
-    Lisp_Object raw_status_low;
-    Lisp_Object raw_status_high;
     /* Symbol indicating status of process.
        This may be a symbol: run, open, or closed.
        Or it may be a list, whose car is stop, exit or signal
@@ -75,10 +69,6 @@
     Lisp_Object status;
     /* Non-nil if communicating through a pty.  */
     Lisp_Object pty_flag;
-    /* Event-count of last event in which this process changed status.  */
-    Lisp_Object tick;
-    /* Event-count of last such event reported.  */
-    Lisp_Object update_tick;
     /* Coding-system for decoding the input from this process.  */
     Lisp_Object decode_coding_system;
     /* Working buffer for decoding.  */
@@ -112,6 +102,21 @@
     Lisp_Object read_output_delay;
     /* Skip reading this process on next read.  */
     Lisp_Object read_output_skip;
+
+    /* After this point, there are no Lisp_Objects any more.  */
+
+    /* Number of this process.
+       allocate_process assumes this is the first non-Lisp_Objects field.
+       A value 0 is used for pseudo-processes such as network connections.  */
+    pid_t pid;
+    /* Record the process status in the raw form in which it comes from `wait'.
+       This is to avoid consing in a signal handler.  */
+    int raw_status_new : 1;
+    int raw_status;
+    /* Event-count of last event in which this process changed status.  */
+    int tick;
+    /* Event-count of last such event reported.  */
+    int update_tick;
 };
 
 /* Every field in the preceding structure except for the first two



--- orig/src/lisp.h
+++ mod/src/lisp.h
@@ -746,6 +746,20 @@
                         + sizeof(Lisp_Object) - 1) /* round up */     \
 		       / sizeof (Lisp_Object))
 
+#ifdef offsetof
+#define OFFSETOF(type,field) offsetof(type,field)
+#else
+#define OFFSETOF(type,field) \
+  ((int)((char*)&((type*)0)->field - (char*)0))
+#endif
+
+/* Like VECSIZE, but used when the pseudo-vector has non-Lisp_Object fields
+   at the end and we need to compute the number of Lisp_Object fields (the
+   ones that the GC needs to trace).  */
+#define SPECVECSIZE(type, nonlispfield) \
+  ((offsetof(type, nonlispfield) - offsetof(struct Lisp_Vector, contents[0])) \
+   / sizeof (Lisp_Object))
+
 struct Lisp_Vector
   {
     EMACS_INT size;


--- orig/src/alloc.c
+++ mod/src/alloc.c
@@ -3069,13 +3069,14 @@
 struct Lisp_Process *
 allocate_process ()
 {
-  EMACS_INT len = VECSIZE (struct Lisp_Process);
-  struct Lisp_Vector *v = allocate_vectorlike (len, MEM_TYPE_PROCESS);
+  EMACS_INT memlen = VECSIZE (struct Lisp_Process);
+  EMACS_INT lisplen = SPECVECSIZE (struct Lisp_Process, pid);
+  struct Lisp_Vector *v = allocate_vectorlike (memlen, MEM_TYPE_PROCESS);
   EMACS_INT i;
 
-  for (i = 0; i < len; ++i)
+  for (i = 0; i < lisplen; ++i)
     v->contents[i] = Qnil;
-  v->size = len;
+  v->size = lisplen;
 
   return (struct Lisp_Process *) v;
 }


--- orig/src/process.c
+++ mod/src/process.c
@@ -415,10 +415,10 @@
      struct Lisp_Process *p;
 {
   union { int i; WAITTYPE wt; } u;
-  u.i = XFASTINT (p->raw_status_low) + (XFASTINT (p->raw_status_high) << 16);
+  eassert (p->raw_status_new);
+  u.i = p->raw_status;
   p->status = status_convert (u.wt);
-  p->raw_status_low = Qnil;
-  p->raw_status_high = Qnil;
+  p->raw_status_new = 0;
 }
 
 /*  Convert a process status word in Unix format to
@@ -620,11 +620,10 @@
 
   XSETINT (p->infd, -1);
   XSETINT (p->outfd, -1);
-  XSETFASTINT (p->pid, 0);
-  XSETFASTINT (p->tick, 0);
-  XSETFASTINT (p->update_tick, 0);
-  p->raw_status_low = Qnil;
-  p->raw_status_high = Qnil;
+  p->pid = 0;
+  p->tick = 0;
+  p->update_tick = 0;
+  p->raw_status_new = 0;
   p->status = Qrun;
   p->mark = Fmake_marker ();
 
@@ -790,12 +789,11 @@
   process = get_process (process);
   p = XPROCESS (process);
 
-  p->raw_status_low = Qnil;
-  p->raw_status_high = Qnil;
+  p->raw_status_new = 0;
   if (NETCONN1_P (p))
     {
       p->status = Fcons (Qexit, Fcons (make_number (0), Qnil));
-      XSETINT (p->tick, ++process_tick);
+      p->tick = ++process_tick;
       status_notify (p);
     }
   else if (XINT (p->infd) >= 0)
@@ -804,7 +802,7 @@
       /* Do this now, since remove_process will make sigchld_handler do nothing.  */
       p->status
 	= Fcons (Qsignal, Fcons (make_number (SIGKILL), Qnil));
-      XSETINT (p->tick, ++process_tick);
+      p->tick = ++process_tick;
       status_notify (p);
     }
   remove_process (process);
@@ -841,7 +839,7 @@
     return process;
 
   p = XPROCESS (process);
-  if (!NILP (p->raw_status_low))
+  if (p->raw_status_new)
     update_status (p);
   status = p->status;
   if (CONSP (status))
@@ -866,7 +864,7 @@
      register Lisp_Object process;
 {
   CHECK_PROCESS (process);
-  if (!NILP (XPROCESS (process)->raw_status_low))
+  if (XPROCESS (process)->raw_status_new)
     update_status (XPROCESS (process));
   if (CONSP (XPROCESS (process)->status))
     return XCAR (XCDR (XPROCESS (process)->status));
@@ -881,7 +879,9 @@
      register Lisp_Object process;
 {
   CHECK_PROCESS (process);
-  return XPROCESS (process)->pid;
+  return (XPROCESS (process)->pid
+	  ? make_fixnum_or_float (XPROCESS (process)->pid)
+	  : Qnil);
 }
 
 DEFUN ("process-name", Fprocess_name, Sprocess_name, 1, 1, 0,
@@ -1363,7 +1363,7 @@
       Finsert (1, &p->name);
       Findent_to (i_status, minspace);
 
-      if (!NILP (p->raw_status_low))
+      if (p->raw_status_new)
 	update_status (p);
       symbol = p->status;
       if (CONSP (p->status))
@@ -1735,7 +1735,7 @@
     abort ();
 
   /* Was PROC started successfully?  */
-  if (XINT (XPROCESS (proc)->pid) <= 0)
+  if (XPROCESS (proc)->pid <= 0)
     remove_process (proc);
 
   return Qnil;
@@ -1946,7 +1946,7 @@
      in the table after this function has returned; if it does
      it might cause call-process to hang and subsequent asynchronous
      processes to get their return values scrambled.  */
-  XSETINT (XPROCESS (process)->pid, -1);
+  XPROCESS (process)->pid = -1;
 
   BLOCK_INPUT;
 
@@ -2137,7 +2137,7 @@
   else
     {
       /* vfork succeeded.  */
-      XSETFASTINT (XPROCESS (process)->pid, pid);
+      XPROCESS (process)->pid = pid;
 
 #ifdef WINDOWSNT
       register_child (pid, inchannel);
@@ -3350,7 +3350,7 @@
     p->kill_without_query = Qt;
   if ((tem = Fplist_get (contact, QCstop), !NILP (tem)))
     p->command = Qt;
-  p->pid = Qnil;
+  p->pid = 0;
   XSETINT (p->infd, inch);
   XSETINT (p->outfd, outch);
   if (is_server && socktype == SOCK_STREAM)
@@ -4066,7 +4066,7 @@
   p->sentinel = ps->sentinel;
   p->filter = ps->filter;
   p->command = Qnil;
-  p->pid = Qnil;
+  p->pid = 0;
   XSETINT (p->infd, s);
   XSETINT (p->outfd, s);
   p->status = Qrun;
@@ -4369,9 +4369,9 @@
 
       /* Don't wait for output from a non-running process.  Just
          read whatever data has already been received.  */
-      if (wait_proc != 0 && !NILP (wait_proc->raw_status_low))
+      if (wait_proc && wait_proc->raw_status_new)
 	update_status (wait_proc);
-      if (wait_proc != 0
+      if (wait_proc
 	  && ! EQ (wait_proc->status, Qrun)
 	  && ! EQ (wait_proc->status, Qconnect))
 	{
@@ -4753,9 +4753,9 @@
 	      else
 		{
 		  /* Preserve status of processes already terminated.  */
-		  XSETINT (XPROCESS (proc)->tick, ++process_tick);
+		  XPROCESS (proc)->tick = ++process_tick;
 		  deactivate_process (proc);
-		  if (!NILP (XPROCESS (proc)->raw_status_low))
+		  if (XPROCESS (proc)->raw_status_new)
 		    update_status (XPROCESS (proc));
 		  if (EQ (XPROCESS (proc)->status, Qrun))
 		    XPROCESS (proc)->status
@@ -4805,7 +4805,7 @@
 #endif
 	      if (xerrno)
 		{
-		  XSETINT (p->tick, ++process_tick);
+		  p->tick = ++process_tick;
 		  p->status = Fcons (Qfailed, Fcons (make_number (xerrno), Qnil));
 		  deactivate_process (proc);
 		}
@@ -5292,7 +5292,7 @@
   VMS_PROC_STUFF *vs, *get_vms_process_pointer();
 #endif /* VMS */
 
-  if (! NILP (p->raw_status_low))
+  if (p->raw_status_new)
     update_status (p);
   if (! EQ (p->status, Qrun))
     error ("Process %s not running", SDATA (p->name));
@@ -5556,10 +5556,9 @@
       proc = process_sent_to;
       p = XPROCESS (proc);
 #endif
-      p->raw_status_low = Qnil;
-      p->raw_status_high = Qnil;
+      p->raw_status_new = 0;
       p->status = Fcons (Qexit, Fcons (make_number (256), Qnil));
-      XSETINT (p->tick, ++process_tick);
+      p->tick = ++process_tick;
       deactivate_process (proc);
 #ifdef VMS
       error ("Error writing to process %s; closed it", SDATA (p->name));
@@ -5672,7 +5671,7 @@
 
   gid = emacs_get_tty_pgrp (p);
 
-  if (gid == XFASTINT (p->pid))
+  if (gid == p->pid)
     return Qnil;
   return Qt;
 }
@@ -5719,7 +5718,7 @@
   /* If we are using pgrps, get a pgrp number and make it negative.  */
   if (NILP (current_group))
     /* Send the signal to the shell's process group.  */
-    gid = XFASTINT (p->pid);
+    gid = p->pid;
   else
     {
 #ifdef SIGNALS_VIA_CHARACTERS
@@ -5838,7 +5837,7 @@
       if (gid == -1)
 	/* If we can't get the information, assume
 	   the shell owns the tty.  */
-	gid = XFASTINT (p->pid);
+	gid = p->pid;
 
       /* It is not clear whether anything really can set GID to -1.
 	 Perhaps on some system one of those ioctls can or could do so.
@@ -5848,12 +5847,12 @@
 #else  /* ! defined (TIOCGPGRP ) */
       /* Can't select pgrps on this system, so we know that
 	 the child itself heads the pgrp.  */
-      gid = XFASTINT (p->pid);
+      gid = p->pid;
 #endif /* ! defined (TIOCGPGRP ) */
 
       /* If current_group is lambda, and the shell owns the terminal,
 	 don't send any signal.  */
-      if (EQ (current_group, Qlambda) && gid == XFASTINT (p->pid))
+      if (EQ (current_group, Qlambda) && gid == p->pid)
 	return;
     }
 
@@ -5861,10 +5860,9 @@
     {
 #ifdef SIGCONT
     case SIGCONT:
-      p->raw_status_low = Qnil;
-      p->raw_status_high = Qnil;
+      p->raw_status_new = 0;
       p->status = Qrun;
-      XSETINT (p->tick, ++process_tick);
+      p->tick = ++process_tick;
       if (!nomsg)
 	status_notify (NULL);
       break;
@@ -5881,7 +5879,7 @@
 #endif
     case SIGKILL:
 #ifdef VMS
-      sys$forcex (&(XFASTINT (p->pid)), 0, 1);
+      sys$forcex (&(p->pid), 0, 1);
       whoosh:
 #endif
       flush_pending_output (XINT (p->infd));
@@ -5893,7 +5891,7 @@
      obvious alternative.  */
   if (no_pgrp)
     {
-      kill (XFASTINT (p->pid), signo);
+      kill (p->pid, signo);
       return;
     }
 
@@ -5906,7 +5904,7 @@
     }
   else
     {
-      gid = - XFASTINT (p->pid);
+      gid = - p->pid;
       kill (gid, signo);
     }
 #else /* ! defined (TIOCSIGSEND) */
@@ -6026,11 +6024,17 @@
      (process, sigcode)
      Lisp_Object process, sigcode;
 {
-  Lisp_Object pid;
+  pid_t pid;
 
   if (INTEGERP (process))
     {
-      pid = process;
+      pid = XINT (process);
+      goto got_it;
+    }
+
+  if (FLOATP (process))
+    {
+      pid = (pid_t) XFLOAT (process);
       goto got_it;
     }
 
@@ -6039,8 +6043,8 @@
       Lisp_Object tem;
       if (tem = Fget_process (process), NILP (tem))
 	{
-	  pid = Fstring_to_number (process, make_number (10));
-	  if (XINT (pid) != 0)
+	  pid = XINT (Fstring_to_number (process, make_number (10)));
+	  if (pid > 0)
 	    goto got_it;
 	}
       process = tem;
@@ -6053,7 +6057,7 @@
 
   CHECK_PROCESS (process);
   pid = XPROCESS (process)->pid;
-  if (!INTEGERP (pid) || XINT (pid) <= 0)
+  if (pid <= 0)
     error ("Cannot signal process %s", SDATA (XPROCESS (process)->name));
 
  got_it:
@@ -6172,7 +6176,7 @@
 
 #undef handle_signal
 
-  return make_number (kill (XINT (pid), XINT (sigcode)));
+  return make_number (kill (pid, XINT (sigcode)));
 }
 
 DEFUN ("process-send-eof", Fprocess_send_eof, Sprocess_send_eof, 0, 1, 0,
@@ -6196,7 +6200,7 @@
   coding = proc_encode_coding_system[XINT (XPROCESS (proc)->outfd)];
 
   /* Make sure the process is really alive.  */
-  if (! NILP (XPROCESS (proc)->raw_status_low))
+  if (XPROCESS (proc)->raw_status_new)
     update_status (XPROCESS (proc));
   if (! EQ (XPROCESS (proc)->status, Qrun))
     error ("Process %s not running", SDATA (XPROCESS (proc)->name));
@@ -6221,7 +6225,7 @@
 	 for communication with the subprocess, call shutdown to cause EOF.
 	 (In some old system, shutdown to socketpair doesn't work.
 	 Then we just can't win.)  */
-      if (NILP (XPROCESS (proc)->pid)
+      if (XPROCESS (proc)->pid == 0
 	  || XINT (XPROCESS (proc)->outfd) == XINT (XPROCESS (proc)->infd))
 	shutdown (XINT (XPROCESS (proc)->outfd), 1);
       /* In case of socketpair, outfd == infd, so don't close it.  */
@@ -6358,7 +6362,7 @@
 	{
 	  proc = XCDR (XCAR (tail));
 	  p = XPROCESS (proc);
-	  if (GC_EQ (p->childp, Qt) && XINT (p->pid) == pid)
+	  if (GC_EQ (p->childp, Qt) && p->pid == pid)
 	    break;
 	  p = 0;
 	}
@@ -6370,7 +6374,7 @@
 	  {
 	    proc = XCDR (XCAR (tail));
 	    p = XPROCESS (proc);
-	    if (GC_INTEGERP (p->pid) && XINT (p->pid) == -1)
+	    if (p->pid == -1)
 	      break;
 	    p = 0;
 	  }
@@ -6381,10 +6385,10 @@
 	  union { int i; WAITTYPE wt; } u;
 	  int clear_desc_flag = 0;
 
-	  XSETINT (p->tick, ++process_tick);
+	  p->tick = ++process_tick;
 	  u.wt = w;
-	  XSETINT (p->raw_status_low, u.i & 0xffff);
-	  XSETINT (p->raw_status_high, u.i >> 16);
+	  p->raw_status = u.i;
+	  p->raw_status_new = 1;
 
 	  /* If process has terminated, stop waiting for its output.  */
 	  if ((WIFSIGNALED (w) || WIFEXITED (w))
@@ -6565,9 +6569,9 @@
       proc = Fcdr (XCAR (tail));
       p = XPROCESS (proc);
 
-      if (XINT (p->tick) != XINT (p->update_tick))
+      if (p->tick != p->update_tick)
 	{
-	  XSETINT (p->update_tick, XINT (p->tick));
+	  p->update_tick = p->tick;
 
 	  /* If process is still active, read any output that remains.  */
 	  while (! EQ (p->filter, Qt)
@@ -6581,7 +6585,7 @@
 	  buffer = p->buffer;
 
 	  /* Get the text to use for the message.  */
-	  if (!NILP (p->raw_status_low))
+	  if (p->raw_status_new)
 	    update_status (p);
 	  msg = status_message (p);
 
@@ -6603,7 +6607,7 @@
 	     So set p->update_tick again
 	     so that an error in the sentinel will not cause
 	     this code to be run again.  */
-	  XSETINT (p->update_tick, XINT (p->tick));
+	  p->update_tick = p->tick;
 	  /* Now output the message suitably.  */
 	  if (!NILP (p->sentinel))
 	    exec_sentinel (proc, msg);

Re: QNX subprocess bug

[Richard Stallman]

It looks correct, except that

+    int raw_status_new : 1;
+    int raw_status;

each of those fields needs to be specifically and precisely documented.

Re: QNX subprocess bug

[Kim F. Storm]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> I think the best solution is to keep pid_t values (without changing them
> into Lisp_Object values) as long as possible, and to use floats when turning
> them into Lisp_Object.

Thanks Stefan.

This is a change that I have wanted to make for a very long time!

However, I would change the following not-really-Lisp_Object fields as well:

         unsigned pty_flag : 1;
           == !NILP (pty_flag)

         unsigned inherit_coding_system_flag : 1;
           == !NILP (inherit_coding_system_flag)

         unsigned read_output_skip : 1;
           == !NILP (read_output_skip)

         int read_output_delay;
           == XINT (read_output_delay)

The adaptive_read_buffering field could be replaced by two new
fields (which would also improve code readability):

       unsigned adaptive_read_buffering : 1;
          == !NILP (Vprocess_adaptive_read_buffering);
         
       unsigned adaptive_read_buffering_clear_on_write : 1;
          == EQ (Vprocess_adaptive_read_buffering, Qt)

I also noticed that the field encoding_carryover is no longer used,
so it can be deleted.

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: QNX subprocess bug

[Stefan Monnier]

>> I think the best solution is to keep pid_t values (without changing them
>> into Lisp_Object values) as long as possible, and to use floats when turning
>> them into Lisp_Object.

> Thanks Stefan.

> This is a change that I have wanted to make for a very long time!

> However, I would change the following not-really-Lisp_Object fields as well:

Although I did move the `tick' vars, the intention was really to stick to
a small patch.  But you're right that many more variables can be moved to
the non-Lisp_Object part.  I just figured that can be moved later on (the
only important one for now is the pid).


        Stefan

Re: QNX subprocess bug

[Richard Stallman]

    However, I would change the following not-really-Lisp_Object fields as well:

Please let's not make this change any bigger than necessary.