Zsh Mailing List Archive
Messages sorted by: Reverse Date, Date, Thread, Author

fix watch implementation



Hi,

when evaluating zsh as the new default shell for interactive use it was
discovered, that the 'watch' feature doesn't work on Solaris 11.3 and
Ubuntu 14.04, 15.04 and probably other modern platforms: 'log' never
shows anything and logins/logouts never show up.

The root cause is, that the current implementation uses a rather
discouraged way to obtain utmp[x] records, i.e. it reads binary records
instead of using the POSIX/recommended way, i.e. {set,get,end}ut[x]ent() -
similar to getpwent(). To check I just inserted a printf: 1st record
seems to be ok but is always of ut_type != USER_PROCESS and all following
records read contain really garbage with ut_type = 0 (probably by
accident). So no wonder, why it doesn't work as expected.

The attached patch fixes the implementation, has been successfully
tested on the platforms mentioned above (was developed against 5.0.7,
which is, what the vendors of the mentioned OS ship, however, someone on
irc reported, that it applies cleanly to the current master version as
well).

Would be nice if it could be merged ASAP. IIRC, we started to evaluate
zsh as default shell 5+ years ago. First test was, whether watch works:
it didn't, but had no time to dig deeper, so we stopped and postponed
the eval ...

have fun,
jel.
-- 
Otto-von-Guericke University     http://www.cs.uni-magdeburg.de/
Department of Computer Science   Geb. 29 R 027, Universitaetsplatz 2
39106 Magdeburg, Germany         Tel: +49 391 67 52768
--- zsh-5.0.7/Src/watch.c.orig	Thu Jul 31 20:41:49 2014
+++ zsh-5.0.7/Src/watch.c	Wed Jan 11 16:37:49 2017
@@ -87,6 +87,9 @@
 
 #if !defined(WATCH_STRUCT_UTMP) && defined(HAVE_STRUCT_UTMPX) && defined(REAL_UTMPX_FILE)
 # define WATCH_STRUCT_UTMP struct utmpx
+# define setutent setutxent
+# define getutent getutxent
+# define endutent endutxent 
 /*
  * In utmpx, the ut_name field is replaced by ut_user.
  * Howver, on some systems ut_name may already be defined this
@@ -141,9 +144,9 @@
 #  define WATCH_WTMP_FILE "/dev/null"
 # endif
 
-static int wtabsz;
-static WATCH_STRUCT_UTMP *wtab;
-static time_t lastutmpcheck;
+static int wtabsz = 0;
+static WATCH_STRUCT_UTMP *wtab = NULL;
+static time_t lastutmpcheck = 0;
 
 /* get the time of login/logout for WATCH */
 
@@ -449,34 +452,44 @@
 /* initialize the user List */
 
 /**/
-static void
-readwtab(void)
+static int
+readwtab(WATCH_STRUCT_UTMP **head, int initial_sz)
 {
-    WATCH_STRUCT_UTMP *uptr;
-    int wtabmax = 32;
-    FILE *in;
+    WATCH_STRUCT_UTMP *uptr, *tmp;
+    int wtabmax = initial_sz < 2 ? 32 : initial_sz;
+    int sz = 0;
 
-    wtabsz = 0;
-    if (!(in = fopen(WATCH_UTMP_FILE, "r")))
-	return;
-    uptr = wtab = (WATCH_STRUCT_UTMP *)zalloc(wtabmax * sizeof(WATCH_STRUCT_UTMP));
-    while (fread(uptr, sizeof(WATCH_STRUCT_UTMP), 1, in))
+
+    uptr = *head = (WATCH_STRUCT_UTMP *)zalloc(wtabmax * sizeof(WATCH_STRUCT_UTMP));
+	setutent();
+    while ((tmp = getutent()) != NULL)
 # ifdef USER_PROCESS
-	if   (uptr->ut_type == USER_PROCESS)
+	if   (tmp->ut_type == USER_PROCESS)
 # else /* !USER_PROCESS */
-	if   (uptr->ut_name[0])
+	if   (tmp->ut_name[0])
 # endif /* !USER_PROCESS */
 	{
+		memcpy(uptr, tmp, sizeof (WATCH_STRUCT_UTMP));
 	    uptr++;
-	    if (++wtabsz == wtabmax)
-		uptr = (wtab = (WATCH_STRUCT_UTMP *)realloc((void *) wtab, (wtabmax *= 2) *
-						      sizeof(WATCH_STRUCT_UTMP))) + wtabsz;
+	    if (++sz == wtabmax) {
+			uptr = (WATCH_STRUCT_UTMP *)
+				realloc(*head, (wtabmax *= 2) * sizeof(WATCH_STRUCT_UTMP));
+			if (uptr == NULL) {
+				/* memory pressure - so stop consuming and use, what we have
+				 * Other option is to exit() here, as zmalloc does on error */
+				sz--;
+				break;
+			}
+			*head = uptr;
+			uptr += sz;
+		}
 	}
-    fclose(in);
+    endutent();
 
-    if (wtabsz)
-	qsort((void *) wtab, wtabsz, sizeof(WATCH_STRUCT_UTMP),
+    if (sz)
+	qsort((void *) *head, sz, sizeof(WATCH_STRUCT_UTMP),
 	           (int (*) _((const void *, const void *)))ucmp);
+	return sz;
 }
 
 /* Check for login/logout events; executed before *
@@ -486,55 +499,28 @@
 void
 dowatch(void)
 {
-    FILE *in;
     WATCH_STRUCT_UTMP *utab, *uptr, *wptr;
     struct stat st;
     char **s;
     char *fmt;
-    int utabsz = 0, utabmax = wtabsz + 4;
-    int uct, wct;
+    int utabsz, uct, wct;
 
     s = watch;
 
     holdintr();
-    if (!wtab) {
-	readwtab();
-	noholdintr();
-	return;
-    }
+    if (!wtab)
+	wtabsz = readwtab(&wtab, 32);
     if ((stat(WATCH_UTMP_FILE, &st) == -1) || (st.st_mtime <= lastutmpcheck)) {
 	noholdintr();
 	return;
     }
     lastutmpcheck = st.st_mtime;
-    uptr = utab = (WATCH_STRUCT_UTMP *) zalloc(utabmax * sizeof(WATCH_STRUCT_UTMP));
-
-    if (!(in = fopen(WATCH_UTMP_FILE, "r"))) {
-	free(utab);
-	noholdintr();
-	return;
-    }
-    while (fread(uptr, sizeof *uptr, 1, in))
-# ifdef USER_PROCESS
-	if (uptr->ut_type == USER_PROCESS)
-# else /* !USER_PROCESS */
-	if (uptr->ut_name[0])
-# endif /* !USER_PROCESS */
-	{
-	    uptr++;
-	    if (++utabsz == utabmax)
-		uptr = (utab = (WATCH_STRUCT_UTMP *)realloc((void *) utab, (utabmax *= 2) *
-						      sizeof(WATCH_STRUCT_UTMP))) + utabsz;
-	}
-    fclose(in);
+    utabsz = readwtab(&utab, wtabsz + 4);
     noholdintr();
     if (errflag) {
 	free(utab);
 	return;
     }
-    if (utabsz)
-	qsort((void *) utab, utabsz, sizeof(WATCH_STRUCT_UTMP),
-	           (int (*) _((const void *, const void *)))ucmp);
 
     wct = wtabsz;
     uct = utabsz;


Messages sorted by: Reverse Date, Date, Thread, Author