Re: [patch] who and stale utmp entries

[Paul Eggert]

Tim Waugh <address@hidden> writes:

> However, perhaps the extra check should be omitted if FILE is
> specified.

OK, thanks for explaining it.  Can you please try the following patch
instead?  It tries to do the right thing in that case.  It also checks
for bogus signal numbers (i.e., nonpositive ones).  I don't have a bad
utmp file to test with, so I'd appreciate it if you'd try it out
on your bad file before I document and install this change.

Index: lib/readutmp.c
===================================================================
RCS file: /fetish/cu/lib/readutmp.c,v
retrieving revision 1.23
diff -p -u -r1.23 readutmp.c
--- lib/readutmp.c      30 Nov 2004 21:39:26 -0000      1.23
+++ lib/readutmp.c      24 Mar 2005 18:42:26 -0000
@@ -26,6 +26,8 @@
 
 #include <sys/types.h>
 #include <sys/stat.h>
+#include <signal.h>
+#include <stdbool.h>
 #include <string.h>
 #include <stdlib.h>
 
@@ -56,19 +58,32 @@ extract_trimmed_name (const STRUCT_UTMP 
   return trimmed_name;
 }
 
+/* Is the utmp entry U desired by the user who asked for OPTIONS?  */
+
+static inline bool
+desirable_utmp_entry (STRUCT_UTMP const *u, int options)
+{
+  return ! (options & READ_UTMP_CHECK_PIDS
+           && (UT_PID (u) <= 0
+               || (kill (UT_PID (u), 0) < 0 && errno == ESRCH)));
+}
+
 /* Read the utmp entries corresponding to file FILENAME into freshly-
    malloc'd storage, set *UTMP_BUF to that pointer, set *N_ENTRIES to
    the number of entries, and return zero.  If there is any error,
-   return -1, setting errno, and don't modify the parameters.  */
+   return -1, setting errno, and don't modify the parameters.
+   If OPTIONS & READ_UTMP_CHECK_PIDS is nonzero, omit entries whose
+   process-IDs do not currently exist.  */
 
 #ifdef UTMP_NAME_FUNCTION
 
 int
-read_utmp (const char *filename, size_t *n_entries, STRUCT_UTMP **utmp_buf)
+read_utmp (const char *filename, size_t *n_entries, STRUCT_UTMP **utmp_buf,
+          int options)
 {
-  size_t n_read;
-  size_t n_alloc = 4;
-  STRUCT_UTMP *utmp = xmalloc (n_alloc * sizeof *utmp);
+  size_t n_read = 0;
+  size_t n_alloc = 0;
+  STRUCT_UTMP *utmp = NULL;
   STRUCT_UTMP *u;
 
   /* Ignore the return value for now.
@@ -79,17 +94,14 @@ read_utmp (const char *filename, size_t 
 
   SET_UTMP_ENT ();
 
-  n_read = 0;
   while ((u = GET_UTMP_ENT ()) != NULL)
-    {
-      if (n_read == n_alloc)
-       {
-         utmp = xnrealloc (utmp, n_alloc, 2 * sizeof *utmp);
-         n_alloc *= 2;
-       }
-      ++n_read;
-      utmp[n_read - 1] = *u;
-    }
+    if (desirable_utmp_entry (u, options))
+      {
+       if (n_read == n_alloc)
+         utmp = x2nrealloc (utmp, &n_alloc, sizeof *utmp);
+
+       utmp[n_read++] = *u;
+      }
 
   END_UTMP_ENT ();
 
@@ -108,6 +120,8 @@ read_utmp (const char *filename, size_t 
   struct stat file_stats;
   size_t n_read;
   size_t size;
+  size_t i;
+  size_t n_desired;
   STRUCT_UTMP *buf;
 
   utmp = fopen (filename, "r");
@@ -140,7 +154,12 @@ read_utmp (const char *filename, size_t 
       return -1;
     }
 
-  *n_entries = n_read;
+  n_desired = 0;
+  for (i = 0; i < n_read; i++)
+    if (desirable_utmp_entry (&utmp[i], options))
+      utmp[n_desired++] = utmp[i];
+
+  *n_entries = n_desired;
   *utmp_buf = buf;
 
   return 0;
Index: lib/readutmp.h
===================================================================
RCS file: /fetish/cu/lib/readutmp.h,v
retrieving revision 1.18
diff -p -u -r1.18 readutmp.h
--- lib/readutmp.h      1 Dec 2004 07:07:38 -0000       1.18
+++ lib/readutmp.h      24 Mar 2005 18:42:26 -0000
@@ -167,6 +167,12 @@ enum { UT_USER_SIZE = sizeof UT_USER ((S
 #  define WTMP_FILE "/etc/wtmp"
 # endif
 
+# if HAVE_STRUCT_XTMP_UT_PID
+#  define UT_PID(U) ((U)->ut_pid)
+# else
+#  define UT_PID(U) 0
+# endif
+
 # if HAVE_STRUCT_UTMP_UT_TYPE || HAVE_STRUCT_UTMPX_UT_TYPE
 #  define UT_TYPE_EQ(U, V) ((U)->ut_type == (V))
 #  define UT_TYPE_NOT_DEFINED 0
@@ -192,7 +198,14 @@ enum { UT_USER_SIZE = sizeof UT_USER ((S
     && (UT_TYPE_USER_PROCESS (U)                               \
         || (UT_TYPE_NOT_DEFINED && UT_TIME_MEMBER (U) != 0)))
 
+/* Options for read_utmp.  */
+enum
+  {
+    READ_UTMP_CHECK_PIDS = 1
+  };
+
 char *extract_trimmed_name (const STRUCT_UTMP *ut);
-int read_utmp (const char *filename, size_t *n_entries, STRUCT_UTMP 
**utmp_buf);
+int read_utmp (const char *filename, size_t *n_entries, STRUCT_UTMP **utmp_buf,
+              int options);
 
 #endif /* __READUTMP_H__ */
Index: src/pinky.c
===================================================================
RCS file: /fetish/cu/src/pinky.c,v
retrieving revision 1.44
diff -p -u -r1.44 pinky.c
--- src/pinky.c 30 Nov 2004 21:40:12 -0000      1.44
+++ src/pinky.c 24 Mar 2005 18:42:27 -0000
@@ -487,7 +487,7 @@ short_pinky (const char *filename,
   size_t n_users;
   STRUCT_UTMP *utmp_buf;
 
-  if (read_utmp (filename, &n_users, &utmp_buf) != 0)
+  if (read_utmp (filename, &n_users, &utmp_buf, 0) != 0)
     error (EXIT_FAILURE, errno, "%s", filename);
 
   scan_entries (n_users, utmp_buf, argc_names, argv_names);
Index: src/uptime.c
===================================================================
RCS file: /fetish/cu/src/uptime.c,v
retrieving revision 1.48
diff -p -u -r1.48 uptime.c
--- src/uptime.c        30 Nov 2004 21:40:29 -0000      1.48
+++ src/uptime.c        24 Mar 2005 18:42:27 -0000
@@ -154,15 +154,16 @@ print_uptime (size_t n, const STRUCT_UTM
 }
 
 /* Display the system uptime and the number of users on the system,
-   according to utmp file FILENAME. */
+   according to utmp file FILENAME.  Use read_utmp OPTIONS to read the
+   utmp file.  */
 
 static void
-uptime (const char *filename)
+uptime (const char *filename, int options)
 {
   size_t n_users;
   STRUCT_UTMP *utmp_buf;
 
-  if (read_utmp (filename, &n_users, &utmp_buf) != 0)
+  if (read_utmp (filename, &n_users, &utmp_buf, options) != 0)
     error (EXIT_FAILURE, errno, "%s", filename);
 
   print_uptime (n_users, utmp_buf);
@@ -211,11 +212,11 @@ main (int argc, char **argv)
   switch (argc - optind)
     {
     case 0:                    /* uptime */
-      uptime (UTMP_FILE);
+      uptime (UTMP_FILE, READ_UTMP_CHECK_PIDS);
       break;
 
     case 1:                    /* uptime <utmp file> */
-      uptime (argv[optind]);
+      uptime (argv[optind], 0);
       break;
 
     default:                   /* lose */
Index: src/users.c
===================================================================
RCS file: /fetish/cu/src/users.c,v
retrieving revision 1.39
diff -p -u -r1.39 users.c
--- src/users.c 30 Nov 2004 21:40:52 -0000      1.39
+++ src/users.c 24 Mar 2005 18:42:27 -0000
@@ -80,15 +80,16 @@ list_entries_users (size_t n, const STRU
   free (u);
 }
 
-/* Display a list of users on the system, according to utmp file FILENAME. */
+/* Display a list of users on the system, according to utmp file FILENAME.
+   Use read_utmp OPTIONS to read FILENAME.  */
 
 static void
-users (const char *filename)
+users (const char *filename, int options)
 {
   size_t n_users;
   STRUCT_UTMP *utmp_buf;
 
-  if (read_utmp (filename, &n_users, &utmp_buf) != 0)
+  if (read_utmp (filename, &n_users, &utmp_buf, options) != 0)
     error (EXIT_FAILURE, errno, "%s", filename);
 
   list_entries_users (n_users, utmp_buf);
@@ -137,11 +138,11 @@ main (int argc, char **argv)
   switch (argc - optind)
     {
     case 0:                    /* users */
-      users (UTMP_FILE);
+      users (UTMP_FILE, READ_UTMP_CHECK_PIDS);
       break;
 
     case 1:                    /* users <utmp file> */
-      users (argv[optind]);
+      users (argv[optind], 0);
       break;
 
     default:                   /* lose */
Index: src/who.c
===================================================================
RCS file: /fetish/cu/src/who.c,v
retrieving revision 1.105
diff -p -u -r1.105 who.c
--- src/who.c   30 Nov 2004 21:41:09 -0000      1.105
+++ src/who.c   24 Mar 2005 18:42:27 -0000
@@ -84,12 +84,10 @@
 #define IDLESTR_LEN 6
 
 #if HAVE_STRUCT_XTMP_UT_PID
-# define UT_PID(U) ((U)->ut_pid)
 # define PIDSTR_DECL_AND_INIT(Var, Utmp_ent) \
   char Var[INT_STRLEN_BOUND (Utmp_ent->ut_pid) + 1]; \
   sprintf (Var, "%ld", (long int) (Utmp_ent->ut_pid))
 #else
-# define UT_PID(U) 0
 # define PIDSTR_DECL_AND_INIT(Var, Utmp_ent) \
   const char *Var = ""
 #endif
@@ -606,14 +604,15 @@ scan_entries (size_t n, const STRUCT_UTM
     }
 }
 
-/* Display a list of who is on the system, according to utmp file filename. */
+/* Display a list of who is on the system, according to utmp file FILENAME.
+   Use read_utmp OPTIONS to read the file.  */
 static void
-who (const char *filename)
+who (const char *filename, int options)
 {
   size_t n_users;
   STRUCT_UTMP *utmp_buf;
 
-  if (read_utmp (filename, &n_users, &utmp_buf) != 0)
+  if (read_utmp (filename, &n_users, &utmp_buf, options) != 0)
     error (EXIT_FAILURE, errno, "%s", filename);
 
   if (short_list)
@@ -810,18 +809,16 @@ main (int argc, char **argv)
 
   switch (argc - optind)
     {
+    case 2:                    /* who <blurf> <glop> */
+      my_line_only = true;
+      /* Fall through.  */
     case -1:
     case 0:                    /* who */
-      who (UTMP_FILE);
+      who (UTMP_FILE, READ_UTMP_CHECK_PIDS);
       break;
 
     case 1:                    /* who <utmp file> */
-      who (argv[optind]);
-      break;
-
-    case 2:                    /* who <blurf> <glop> */
-      my_line_only = true;
-      who (UTMP_FILE);
+      who (argv[optind], 0);
       break;
 
     default:                   /* lose */