emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: call-process should not block process filters from running


From: Po Lu
Subject: Re: call-process should not block process filters from running
Date: Tue, 04 Jul 2023 12:09:14 +0800
User-agent: Gnus/5.13 (Gnus v5.13)

sbaugh@catern.com writes:

>  /* Clean up files, file descriptors and processes created by Fcall_process.  
> */

This is a stray comment.

> +struct synch_process {
> +  /* If nonzero, a process-ID that has not been reaped.  */
> +  pid_t pid;
> +  /* If a string, the name of a temp file that has not been removed.  */
> +  Lisp_Object tempfile;
> +  int* callproc_fd;
> +  Lisp_Object buffer;
> +};

Please place the opening brace of this structure definition on a new
line, fill the comments above each field, and write:

  int *callproc_fd;

instead of

  int* callproc_fd;

ISTM that this field could be named `fd' instead.

> +  struct synch_process synch_process = {
> +    .pid = 0,
> +    .tempfile = make_fixnum (0),
> +    .callproc_fd = callproc_fd,
> +    .buffer = Fcurrent_buffer (),
> +  };

Why not declare `sync_process' earlier?  The code looks better that way.

> +           wait_reading_process_output(-1, -1, 0, display_on_the_fly,
> +                                       NULL, NULL, 0);
> +           swallow_events(display_on_the_fly);

Please place a space between the function identifier and its parameter
list.

>             if (display_on_the_fly)
>               break;
>           }
> @@ -904,7 +897,7 @@ call_process (ptrdiff_t nargs, Lisp_Object *args, int 
> filefd,
>  
>    /* Don't kill any children that the subprocess may have left behind
>       when exiting.  */
> -  synch_process_pid = 0;
> +  synch_process.pid = 0;
>  
>    SAFE_FREE_UNBIND_TO (count, Qnil);
>  
> @@ -2035,11 +2028,6 @@ syms_of_callproc (void)
>  #endif
>    staticpro (&Vtemp_file_name_pattern);
>  
> -#ifdef MSDOS
> -  synch_process_tempfile = make_fixnum (0);
> -  staticpro (&synch_process_tempfile);
> -#endif

Please make your modifications conditional on _not_ building the MS-DOS
port: as it is a single process ``operating system'', Emacs does not run
while a subprocess is running, so these changes are unnecessary.

I've also pointed out a general issue with this change: when a timer or
selection converter calls `call-process', it does not expect more timers
to run from within.  Assume that a timer periodically calls `rm
/var/run/foo', and it takes a few seconds for `/bin/rm' to page in,
enough for that timer to run again.  Now, one of the calls to `rm' will
fail and signal an error.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]