bug#71477: 30.0.50; Lock files are not deleted on Windows 98

[Paul Eggert <eggert@cs.ucla.edu>]

[-- Attachment #1: Type: text/plain, Size: 1649 bytes --]

On 2024-06-12 01:25, Eli Zaretskii wrote:

> -  if (! c_isdigit (dot[1]))
> +  if (! (c_isdigit (dot[1])
> +	 /* Windows 9X report negative PID values.  */
> +	 || (dot[1] == '-' && c_isdigit (dot[2]))))

Faster is "if (! c_isdigit[(dot[1] == '-') + 1])", as it avoids a 
conditional branch on most platforms.


> -      else if (0 < pid && pid <= TYPE_MAXIMUM (pid_t)
> +      else if (pid != -1 && pid <= TYPE_MAXIMUM (pid_t)
>                  && (kill (pid, 0) >= 0 || errno == EPERM)

This looks dubious for most systems, where 'kill' has special behavior 
when pid < -1 or pid == 0; it tests a process group. That's not the test 
we want here, since we want to check whether Emacs can be sent a signal, 
not whether any process in its process group can be sent a signal (this 
can be valid even after Emacs has exited). The code should use calls 
like kill (-2, 0) and kill (0, 0) only on platforms where we know the 
calls do not test a process group.

Even on MS Windows 98 we should check that TYPE_MINIMUM (pid_t) <= pid. 
Also, is there a special meaning for kill (0, 0) on MS Windows 98? If 
so, we should also check that pid != 0.

Do any MS-Windows platforms support process groups, i.e., kill (-2, 0) 
operates on process group 2 rather than on an individual process with 
process ID -2? If so, these platforms should be careful too, and should 
not use kill (-2, 0) or kill (0, 0).

How about the attached patch instead? You can adjust the 
Microsoft-specific .h files to define VALID_PROCESS_ID appropriately for 
MS Windows 98, and for any other MS platform where kill (-2, 0) is known 
to check for just the individual process -2.

[-- Attachment #2: 0001-Start-of-a-fix-for-bug-71477.patch --]
[-- Type: text/x-patch, Size: 2525 bytes --]

From f5dcecb833c716995d38f90abc6d120d6f5e3064 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Wed, 12 Jun 2024 08:42:24 -0700
Subject: [PATCH] Start of a fix for bug#71477

* src/filelock.c (integer_prefixed): New static function.
(VALID_PROCESS_ID): New macro.
(current_lock_owner): Use them to allow negative process IDs
on some Microsoft platforms.
---
 src/filelock.c | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/src/filelock.c b/src/filelock.c
index 050cac565c9..d4a4747955a 100644
--- a/src/filelock.c
+++ b/src/filelock.c
@@ -342,6 +342,22 @@ read_lock_data (char *lfname, char lfinfo[MAX_LFINFO + 1])
   return nbytes;
 }
 
+/* Whether the string S starts with a decimal integer, optionally
+   negative.  */
+static bool
+integer_prefixed (char const *s)
+{
+  /* Doing it this way avoids a conditional branch on most platforms.  */
+  return c_isdigit (s[s[0] == '-']);
+}
+
+/* Whether the integer P could identify an individual process.  On most
+   platforms this simply checks for positive pid_t, but on some
+   Microsoft ports our headers #define it to to some other test.  */
+#ifndef VALID_PROCESS_ID
+# define VALID_PROCESS_ID(p) (0 < (p) && (p) <= TYPE_MAXIMUM (pid_t))
+#endif
+
 /* True if errno values are negative.  Although the C standard
    requires them to be positive, they are negative in Haiku.  */
 enum { NEGATIVE_ERRNO = EDOM < 0 };
@@ -393,7 +409,7 @@ current_lock_owner (lock_info_type *owner, Lisp_Object lfname)
     return EINVAL;
 
   /* The PID is everything from the last '.' to the ':' or equivalent.  */
-  if (! c_isdigit (dot[1]))
+  if (! integer_prefixed (dot + 1))
     return EINVAL;
   errno = 0;
   pid = strtoimax (dot + 1, &owner->colon, 10);
@@ -419,9 +435,7 @@ current_lock_owner (lock_info_type *owner, Lisp_Object lfname)
       boot += 2;
       FALLTHROUGH;
     case ':':
-      if (!(c_isdigit (boot[0])
-	    /* A negative number.  */
-	    || (boot[0] == '-' && c_isdigit (boot[1]))))
+      if (! integer_prefixed (boot))
 	return EINVAL;
       boot_time = strtoimax (boot, &lfinfo_end, 10);
       break;
@@ -451,7 +465,7 @@ current_lock_owner (lock_info_type *owner, Lisp_Object lfname)
     {
       if (pid == getpid ())
         return I_OWN_IT;
-      else if (0 < pid && pid <= TYPE_MAXIMUM (pid_t)
+      else if (VALID_PROCESS_ID (pid)
                && (kill (pid, 0) >= 0 || errno == EPERM)
 	       && (boot_time == 0
 		   || (boot_time <= TYPE_MAXIMUM (time_t)
-- 
2.43.0