PATCH: separate watch/log functionality out into a module

[Oliver Kiddle <opk@xxxxxxx>]

The log builtin and watch variable likely don't get as much use as they
did in the days of shared servers at Universities. On one particular
Linux system, I can get it to crash with a variable, descriptively
named bv, being null. On FreeBSD it only prints "not available on this
system". Though it doesn't seem to work from tcsh on FreeBSD either –
it originates as a tcsh feature.

This patch extracts the functionality out into a zsh/watch module. I've
set load=yes in the .mdd file so anyone who uses the feature doesn't
need to add zmodload commands to their .zshrc. $watch/$WATCH and the log
builtin are autoloadable features. The variables are tied which needed a
little extra setup. I was concerned that it'd break things if one or
other of them are disabled with zmodload but it seems that if you
disable either one, both end up missing. That's not especially useful
but I didn't want it crashing. Maybe someone else can see a way in which
this setup could be a problem?

WATCHFMT and LOGCHECK are merely set to the defaults if not already set.
That avoids loading the module for anyone like me who sets them in
.zshrc but only conditionally sets $watch on particular hosts.

This change could result in pre-prompt actions running in a different
order but the advantage is that a user perhaps has more control by
deciding which of sched and watch to load first.

Oliver

diff --git a/Src/watch.c b/Src/Modules/watch.c
similarity index 87%
rename from Src/watch.c
rename to Src/Modules/watch.c
index c41704315..02f0562fc 100644
--- a/Src/watch.c
+++ b/Src/Modules/watch.c
@@ -27,7 +27,7 @@
  *
  */
 
-#include "zsh.mdh"
+#include "watch.mdh"
 
 /* Headers for utmp/utmpx structures */
 #ifdef HAVE_UTMP_H
@@ -139,9 +139,6 @@
 # define DEFAULT_WATCHFMT "%n has %a %l."
 #endif /* !WATCH_UTMP_UT_HOST */
 
-/**/
-char const * const default_watchfmt = DEFAULT_WATCHFMT;
-
 #ifdef WATCH_STRUCT_UTMP
 
 # include "watch.pro"
@@ -152,11 +149,14 @@ char const * const default_watchfmt = DEFAULT_WATCHFMT;
 
 static int wtabsz = 0;
 static WATCH_STRUCT_UTMP *wtab = NULL;
+
+/* the last time we checked the people in the WATCH variable */
+static time_t lastwatch;
+
 static time_t lastutmpcheck = 0;
 
 /* get the time of login/logout for WATCH */
 
-/**/
 static time_t
 getlogtime(WATCH_STRUCT_UTMP *u, int inout)
 {
@@ -202,7 +202,6 @@ getlogtime(WATCH_STRUCT_UTMP *u, int inout)
 # define BEGIN3 '('
 # define END3 ')'
 
-/**/
 static char *
 watch3ary(int inout, WATCH_STRUCT_UTMP *u, char *fmt, int prnt)
 {
@@ -407,7 +406,6 @@ watchlog_match(char *teststr, char *actual, int len)
 
 /* check the List for login/logouts */
 
-/**/
 static void
 watchlog(int inout, WATCH_STRUCT_UTMP *u, char **w, char *fmt)
 {
@@ -470,7 +468,6 @@ watchlog(int inout, WATCH_STRUCT_UTMP *u, char **w, char *fmt)
 
 /* compare 2 utmp entries */
 
-/**/
 static int
 ucmp(WATCH_STRUCT_UTMP *u, WATCH_STRUCT_UTMP *v)
 {
@@ -481,7 +478,6 @@ ucmp(WATCH_STRUCT_UTMP *u, WATCH_STRUCT_UTMP *v)
 
 /* initialize the user List */
 
-/**/
 static int
 readwtab(WATCH_STRUCT_UTMP **head, int initial_sz)
 {
@@ -592,10 +588,19 @@ dowatch(void)
     wtab = utab;
     wtabsz = utabsz;
     fflush(stdout);
+    lastwatch = time(NULL);
+}
+
+static void
+checksched(void)
+{
+    /* Do nothing if WATCH is not set, or LOGCHECK has not elapsed */
+    if (watch && (int) difftime(time(NULL), lastwatch) > getiparam("LOGCHECK"))
+	dowatch();
 }
 
 /**/
-int
+static int
 bin_log(UNUSED(char *nam), UNUSED(char **argv), UNUSED(Options ops), UNUSED(int func))
 {
     if (!watch)
@@ -611,16 +616,101 @@ bin_log(UNUSED(char *nam), UNUSED(char **argv), UNUSED(Options ops), UNUSED(int
 
 #else /* !WATCH_STRUCT_UTMP */
 
-/**/
-void dowatch(void)
+static void
+checksched(void)
 {
 }
 
 /**/
-int
+static int
 bin_log(char *nam, char **argv, Options ops, int func)
 {
     return bin_notavail(nam, argv, ops, func);
 }
 
 #endif /* !WATCH_STRUCT_UTMP */
+
+/**/
+static char **watch; /* $watch */
+
+/* module setup */
+
+static struct builtin bintab[] = {
+    BUILTIN("log", 0, bin_log, 0, 0, 0, NULL, NULL),
+};
+
+static struct paramdef partab[] = {
+    PARAMDEF("WATCH", PM_TIED|PM_SCALAR|PM_SPECIAL, &watch, &colonarr_gsu),
+    PARAMDEF("watch", PM_TIED|PM_ARRAY|PM_SPECIAL, &watch, &vararray_gsu),
+};
+
+static struct features module_features = {
+    bintab, sizeof(bintab)/sizeof(*bintab),
+    NULL, 0,
+    NULL, 0,
+    partab, sizeof(partab)/sizeof(*partab),
+    0
+};
+
+/**/
+int
+setup_(UNUSED(Module m))
+{
+    return 0;
+}
+
+/**/
+int
+features_(Module m, char ***features)
+{
+    *features = featuresarray(m, &module_features);
+    return 0;
+}
+
+/**/
+int
+enables_(Module m, int **enables)
+{
+    return handlefeatures(m, &module_features, enables);
+}
+
+/**/
+int
+boot_(UNUSED(Module m))
+{
+    static char const * const default_watchfmt = DEFAULT_WATCHFMT;
+    Param pm;
+
+    if ((pm = (Param) paramtab->getnode(paramtab, "watch")))
+	pm->ename = "WATCH";
+    if ((pm = (Param) paramtab->getnode(paramtab, "WATCH")))
+	pm->ename = "watch";
+    watch = mkarray(NULL);
+
+    /* These two parameters are only set to defaults if not set.
+     * So setting them in .zshrc will not be enough to load the
+     * module. It's useless until the watch array is set anyway. */
+    if (!paramtab->getnode(paramtab, "WATCHFMT"))
+	setsparam("WATCHFMT", ztrdup_metafy(default_watchfmt));
+    if (!paramtab->getnode(paramtab, "LOGCHECK"))
+	setiparam("LOGCHECK", 60);
+
+    addprepromptfn(&checksched);
+
+    return 0;
+}
+
+/**/
+int
+cleanup_(Module m)
+{
+    delprepromptfn(&checksched);
+    return setfeatureenables(m, &module_features, NULL);
+}
+
+/**/
+int
+finish_(UNUSED(Module m))
+{
+    return 0;
+}
diff --git a/Src/Modules/watch.mdd b/Src/Modules/watch.mdd
new file mode 100644
index 000000000..7e8454ede
--- /dev/null
+++ b/Src/Modules/watch.mdd
@@ -0,0 +1,7 @@
+name=zsh/watch
+link=dynamic
+load=yes
+
+autofeatures="b:log p:WATCH p:watch"
+
+objects="watch.o"
diff --git a/Src/builtin.c b/Src/builtin.c
index 89bcd98db..8ef678b22 100644
--- a/Src/builtin.c
+++ b/Src/builtin.c
@@ -89,7 +89,6 @@ static struct builtin builtins[] =
     BUILTIN("kill", BINF_HANDLES_OPTS, bin_kill, 0, -1, 0, NULL, NULL),
     BUILTIN("let", 0, bin_let, 1, -1, 0, NULL, NULL),
     BUILTIN("local", BINF_PLUSOPTS | BINF_MAGICEQUALS | BINF_PSPECIAL | BINF_ASSIGN, (HandlerFunc)bin_typeset, 0, -1, 0, "AE:%F:%HL:%R:%TUZ:%ahi:%lp:%rtux", NULL),
-    BUILTIN("log", 0, bin_log, 0, 0, 0, NULL, NULL),
     BUILTIN("logout", 0, bin_break, 0, 1, BIN_LOGOUT, NULL, NULL),
 
 #if defined(ZSH_MEM) & defined(ZSH_MEM_DEBUG)
diff --git a/Src/init.c b/Src/init.c
index 878a53a37..871d46b12 100644
--- a/Src/init.c
+++ b/Src/init.c
@@ -1042,7 +1042,6 @@ setupvals(char *cmd, char *runscript, char *zsh_name)
 #endif /* FPATH_NEEDS_INIT */
 
     mailpath = mkarray(NULL);
-    watch    = mkarray(NULL);
     psvar    = mkarray(NULL);
     module_path = mkarray(ztrdup(MODULE_DIR));
     modulestab = newmoduletable(17, "modules");
diff --git a/Src/params.c b/Src/params.c
index b703a97ce..dadf83129 100644
--- a/Src/params.c
+++ b/Src/params.c
@@ -63,7 +63,6 @@ char **pparams,		/* $argv        */
      **mailpath,	/* $mailpath    */
      **manpath,		/* $manpath     */
      **psvar,		/* $psvar       */
-     **watch,		/* $watch       */
      **zsh_eval_context; /* $zsh_eval_context */
 /**/
 mod_export
@@ -194,6 +193,10 @@ mod_export const struct gsu_hash stdhash_gsu =
 mod_export const struct gsu_hash nullsethash_gsu =
 { hashgetfn, nullsethashfn, nullunsetfn };
 
+/**/
+mod_export const struct gsu_scalar colonarr_gsu =
+{ colonarrgetfn, colonarrsetfn, stdunsetfn };
+
 
 /* Non standard methods (not exported) */
 static const struct gsu_integer pound_gsu =
@@ -259,9 +262,6 @@ static const struct gsu_integer varint_readonly_gsu =
 static const struct gsu_integer zlevar_gsu =
 { intvargetfn, zlevarsetfn, stdunsetfn };
 
-static const struct gsu_scalar colonarr_gsu =
-{ colonarrgetfn, colonarrsetfn, stdunsetfn };
-
 static const struct gsu_integer argc_gsu =
 { poundgetfn, nullintsetfn, stdunsetfn };
 static const struct gsu_array pipestatus_gsu =
@@ -398,7 +398,6 @@ IPDEF8("CDPATH", &cdpath, "cdpath", PM_TIED),
 IPDEF8("FIGNORE", &fignore, "fignore", PM_TIED),
 IPDEF8("FPATH", &fpath, "fpath", PM_TIED),
 IPDEF8("MAILPATH", &mailpath, "mailpath", PM_TIED),
-IPDEF8("WATCH", &watch, "watch", PM_TIED),
 IPDEF8("PATH", &path, "path", PM_RESTRICTED|PM_TIED),
 IPDEF8("PSVAR", &psvar, "psvar", PM_TIED),
 IPDEF8("ZSH_EVAL_CONTEXT", &zsh_eval_context, "zsh_eval_context", PM_READONLY_SPECIAL|PM_TIED),
@@ -430,7 +429,6 @@ IPDEF9("fpath", &fpath, "FPATH", PM_TIED),
 IPDEF9("mailpath", &mailpath, "MAILPATH", PM_TIED),
 IPDEF9("manpath", &manpath, "MANPATH", PM_TIED),
 IPDEF9("psvar", &psvar, "PSVAR", PM_TIED),
-IPDEF9("watch", &watch, "WATCH", PM_TIED),
 
 IPDEF9("zsh_eval_context", &zsh_eval_context, "ZSH_EVAL_CONTEXT", PM_TIED|PM_READONLY_SPECIAL),
 
@@ -453,7 +451,6 @@ IPDEF8("CDPATH", &cdpath, NULL, 0),
 IPDEF8("FIGNORE", &fignore, NULL, 0),
 IPDEF8("FPATH", &fpath, NULL, 0),
 IPDEF8("MAILPATH", &mailpath, NULL, 0),
-IPDEF8("WATCH", &watch, NULL, 0),
 IPDEF8("PATH", &path, NULL, PM_RESTRICTED),
 IPDEF8("PSVAR", &psvar, NULL, 0),
 IPDEF8("ZSH_EVAL_CONTEXT", &zsh_eval_context, NULL, PM_READONLY_SPECIAL),
@@ -836,7 +833,6 @@ createparamtable(void)
      */
     setsparam("TMPPREFIX", ztrdup_metafy(DEFAULT_TMPPREFIX));
     setsparam("TIMEFMT", ztrdup_metafy(DEFAULT_TIMEFMT));
-    setsparam("WATCHFMT", ztrdup_metafy(default_watchfmt));
 
     hostnam = (char *)zalloc(256);
     gethostname(hostnam, 256);
@@ -4093,7 +4089,7 @@ arrvarsetfn(Param pm, char **x)
 }
 
 /**/
-char *
+mod_export char *
 colonarrgetfn(Param pm)
 {
     char ***dptr = (char ***)pm->u.data;
@@ -4101,7 +4097,7 @@ colonarrgetfn(Param pm)
 }
 
 /**/
-void
+mod_export void
 colonarrsetfn(Param pm, char *x)
 {
     char ***dptr = (char ***)pm->u.data;
diff --git a/Src/utils.c b/Src/utils.c
index ed3690172..8adab2bd7 100644
--- a/Src/utils.c
+++ b/Src/utils.c
@@ -1494,11 +1494,6 @@ deltimedfn(voidvoidfnptr_t func)
 /**/
 time_t lastmailcheck;
 
-/* the last time we checked the people in the WATCH variable */
-
-/**/
-time_t lastwatch;
-
 /*
  * Call a function given by "name" with optional arguments
  * "lnklst".  If these are present the first argument is the function name.
@@ -1637,17 +1632,6 @@ preprompt(void)
     if (errflag)
 	return;
 
-    /* If WATCH is set, then check for the *
-     * specified login/logout events.      */
-    if (watch) {
-	if ((int) difftime(time(NULL), lastwatch) > getiparam("LOGCHECK")) {
-	    dowatch();
-	    lastwatch = time(NULL);
-	}
-    }
-    if (errflag)
-	return;
-
     /* Check mail */
     currentmailcheck = time(NULL);
     if (mailcheck &&
diff --git a/Src/zsh.mdd b/Src/zsh.mdd
index 9bcaccae5..da8d58322 100644
--- a/Src/zsh.mdd
+++ b/Src/zsh.mdd
@@ -13,7 +13,7 @@ objects="builtin.o compat.o cond.o context.o \
 exec.o glob.o hashtable.o hashnameddir.o \
 hist.o init.o input.o jobs.o lex.o linklist.o loop.o math.o \
 mem.o module.o options.o params.o parse.o pattern.o prompt.o signals.o \
-signames.o sort.o string.o subst.o text.o utils.o watch.o \
+signames.o sort.o string.o subst.o text.o utils.o \
 openssh_bsd_setres_id.o"
 
 headers="../config.h zsh_system.h zsh.h sigcount.h signals.h \