[Nmh-workers] Decoupling done()

[Joel Reicher]

Some of you may remember a while ago I wrote about replacing done() with
atexit() registration.

Well, after wasting a lot of time figuring out a good way to do this I've
decided to go with something else, at least to begin with.

The general nature of the problem is as follows. There is a convention in
a lot (not all!) of the nmh code to call done() to exit, and there is
a done() defined in libmh.a which does nothing other than exit().

Some of the nmh executables, however, define their own version of done()
which overrides the one in libmh.a when linking.  Since done() is called
by functions in libmh.a this is effectively a hack to do a callback.

Ultimately these callbacks should probably be replaced by a more
sensible mechanism wherever possible, but each "local" done() has its
own problems to be solved, and I think it's too big and complex a change
to do all at once.

So, as a first step, what I've decided to do is get rid of this link
hack. The patch which follows this email, and which I'd like some comments
on, replaces done(), the function prototype, with a function pointer
called "done", so that all the calls continue to work as is. The libmh.a
version of done is renamed to default_done, and the function pointer is
initialised to point to this function.

Wherever there's a local done() it is renamed in what I think is a sensible
way and in most cases the main() for that executable reassigns the done
function pointer to point to this renamed local done(). A notable exception
is sendsbr.c where I have made a chance which I think is more faithful
to the use of the callback there.

Comments are very welcome. Even better would be some patching and testing.
I'm going to give this a few days (I'm running it on my own system
already) and then I'll commit it to the repository.

Cheers,

        - Joel

Index: h/mh.h
===================================================================
RCS file: /sources/nmh/nmh/h/mh.h,v
retrieving revision 1.6
diff -u -r1.6 mh.h
--- h/mh.h      21 Feb 2006 03:58:31 -0000      1.6
+++ h/mh.h      30 Oct 2007 04:54:57 -0000
@@ -336,5 +336,7 @@
 extern char *whatnowproc;
 extern char *whomproc;
 
+extern int (*done) (int);
+
 #include <h/prototypes.h>
 
Index: h/prototypes.h
===================================================================
RCS file: /sources/nmh/nmh/h/prototypes.h,v
retrieving revision 1.18
diff -u -r1.18 prototypes.h
--- h/prototypes.h      13 Apr 2007 11:53:08 -0000      1.18
+++ h/prototypes.h      30 Oct 2007 04:54:57 -0000
@@ -47,7 +47,7 @@
 void cpydgst (int, int, char *, char *);
 int decode_rfc2047 (char *, char *, size_t);
 void discard (FILE *);
-int done (int);
+int default_done (int);
 int ext_hook(char *, char *, char *);
 int fdcompare (int, int);
 int folder_addmsg (struct msgs **, char *, int, int, int, int, char *);
Index: sbr/done.c
===================================================================
RCS file: /sources/nmh/nmh/sbr/done.c,v
retrieving revision 1.3
diff -u -r1.3 done.c
--- sbr/done.c  2 Jul 2002 22:09:14 -0000       1.3
+++ sbr/done.c  30 Oct 2007 04:54:57 -0000
@@ -11,8 +11,10 @@
 
 #include <h/mh.h>
 
+int (*done) (int) = default_done;
+
 int
-done (int status)
+default_done (int status)
 {
     exit (status);
     return 1;  /* dead code to satisfy the compiler */
Index: uip/inc.c
===================================================================
RCS file: /sources/nmh/nmh/uip/inc.c,v
retrieving revision 1.24
diff -u -r1.24 inc.c
--- uip/inc.c   21 Aug 2007 21:19:39 -0000      1.24
+++ uip/inc.c   30 Oct 2007 04:54:59 -0000
@@ -223,8 +223,8 @@
  */
 char *map_name(char *);
 
+static int inc_done(int);
 #ifdef POP
-int done(int);
 static int pop_action(char *);
 static int pop_pack(char *);
 static int map_count(void);
@@ -263,6 +263,8 @@
     struct hes_postoffice *po;
 #endif
 
+    done=inc_done;
+
 /* absolutely the first thing we do is save our privileges,
  * and drop them if we can.
  */
@@ -987,8 +989,8 @@
 #endif /* if 0 */
 
 
-int
-done (int status)
+static int
+inc_done (int status)
 {
 #ifdef POP
     if (packfile && pd != NOTOK)
Index: uip/mhbuild.c
===================================================================
RCS file: /sources/nmh/nmh/uip/mhbuild.c,v
retrieving revision 1.10
diff -u -r1.10 mhbuild.c
--- uip/mhbuild.c       31 Jan 2006 02:50:57 -0000      1.10
+++ uip/mhbuild.c       30 Oct 2007 04:54:59 -0000
@@ -100,6 +100,7 @@
 static char outfile[BUFSIZ];
 static int unlink_outfile = 0;
 
+static int unlink_done (int);
 
 /* mhbuildsbr.c */
 CT build_mime (char *);
@@ -126,6 +127,8 @@
     CT ct, cts[2];
     FILE *fp;
 
+    done=unlink_done;
+
 #ifdef LOCALE
     setlocale(LC_ALL, "");
 #endif
@@ -382,8 +385,8 @@
 }
 
 
-int
-done (int status)
+static int
+unlink_done (int status)
 {
     /*
      * Check if we need to remove stray
Index: uip/mhlist.c
===================================================================
RCS file: /sources/nmh/nmh/uip/mhlist.c,v
retrieving revision 1.12
diff -u -r1.12 mhlist.c
--- uip/mhlist.c        27 Apr 2006 12:00:28 -0000      1.12
+++ uip/mhlist.c        30 Oct 2007 04:54:59 -0000
@@ -117,6 +117,7 @@
  * static prototypes
  */
 static RETSIGTYPE pipeser (int);
+static int freectp_done (int);
 
 
 int
@@ -131,6 +132,8 @@
     struct msgs *mp = NULL;
     CT ct, *ctp;
 
+    done=freectp_done;
+
 #ifdef LOCALE
     setlocale(LC_ALL, "");
 #endif
@@ -391,8 +394,8 @@
 }
 
 
-int
-done (int status)
+static int
+freectp_done (int status)
 {
     CT *ctp;
 
Index: uip/mhn.c
===================================================================
RCS file: /sources/nmh/nmh/uip/mhn.c,v
retrieving revision 1.12
diff -u -r1.12 mhn.c
--- uip/mhn.c   27 Apr 2006 12:00:28 -0000      1.12
+++ uip/mhn.c   30 Oct 2007 04:55:00 -0000
@@ -205,6 +205,7 @@
  * static prototypes
  */
 static RETSIGTYPE pipeser (int);
+static int freectp_done (int);
 
 
 int
@@ -220,6 +221,8 @@
     CT ct, *ctp;
     FILE *fp;
 
+    done=freectp_done;
+
 #ifdef LOCALE
     setlocale(LC_ALL, "");
 #endif
@@ -704,8 +707,8 @@
 }
 
 
-int
-done (int status)
+static int
+freectp_done (int status)
 {
     CT *ctp;
 
Index: uip/mhshow.c
===================================================================
RCS file: /sources/nmh/nmh/uip/mhshow.c,v
retrieving revision 1.12
diff -u -r1.12 mhshow.c
--- uip/mhshow.c        27 Apr 2006 12:00:28 -0000      1.12
+++ uip/mhshow.c        30 Oct 2007 04:55:00 -0000
@@ -133,6 +133,7 @@
  * static prototypes
  */
 static RETSIGTYPE pipeser (int);
+static int freectp_done (int);
 
 
 int
@@ -147,6 +148,8 @@
     CT ct, *ctp;
     FILE *fp;
 
+    done=freectp_done;
+
 #ifdef LOCALE
     setlocale(LC_ALL, "");
 #endif
@@ -471,8 +474,8 @@
 }
 
 
-int
-done (int status)
+static int
+freectp_done (int status)
 {
     CT *ctp;
 
Index: uip/mhstore.c
===================================================================
RCS file: /sources/nmh/nmh/uip/mhstore.c,v
retrieving revision 1.12
diff -u -r1.12 mhstore.c
--- uip/mhstore.c       27 Apr 2006 12:00:28 -0000      1.12
+++ uip/mhstore.c       30 Oct 2007 04:55:00 -0000
@@ -111,6 +111,7 @@
  * static prototypes
  */
 static RETSIGTYPE pipeser (int);
+static int freectp_done (int);
 
 
 int
@@ -125,6 +126,8 @@
     CT ct, *ctp;
     FILE *fp;
 
+    done=freectp_done;
+
 #ifdef LOCALE
     setlocale(LC_ALL, "");
 #endif
@@ -403,8 +406,8 @@
 }
 
 
-int
-done (int status)
+static int
+freectp_done (int status)
 {
     CT *ctp;
 
Index: uip/mhtest.c
===================================================================
RCS file: /sources/nmh/nmh/uip/mhtest.c,v
retrieving revision 1.12
diff -u -r1.12 mhtest.c
--- uip/mhtest.c        27 Apr 2006 12:00:28 -0000      1.12
+++ uip/mhtest.c        30 Oct 2007 04:55:01 -0000
@@ -114,6 +114,7 @@
  */
 static int write_content (CT *, char *);
 static RETSIGTYPE pipeser (int);
+static int freectp_done (int);
 
 
 int
@@ -127,6 +128,8 @@
     struct msgs *mp = NULL;
     CT ct, *ctp;
 
+    done=freectp_done;
+
 #ifdef LOCALE
     setlocale(LC_ALL, "");
 #endif
@@ -397,8 +400,8 @@
 }
 
 
-int
-done (int status)
+static int
+freectp_done (int status)
 {
     CT *ctp;
 
Index: uip/packf.c
===================================================================
RCS file: /sources/nmh/nmh/uip/packf.c,v
retrieving revision 1.9
diff -u -r1.9 packf.c
--- uip/packf.c 27 Apr 2006 12:00:28 -0000      1.9
+++ uip/packf.c 30 Oct 2007 04:55:01 -0000
@@ -33,6 +33,8 @@
 static int mbx_style = MBOX_FORMAT;
 static int mapping = 0;
 
+static int mbxclose_done(int);
+
 char *file = NULL;
 
 
@@ -46,6 +48,8 @@
     struct msgs *mp;
     struct stat st;
 
+    done=mbxclose_done;
+
 #ifdef LOCALE
     setlocale(LC_ALL, "");
 #endif
@@ -178,8 +182,8 @@
     return done (0);
 }
 
-int
-done (int status)
+static int
+mbxclose_done (int status)
 {
     mbx_close (file, md);
     exit (status);
Index: uip/pick.c
===================================================================
RCS file: /sources/nmh/nmh/uip/pick.c,v
retrieving revision 1.10
diff -u -r1.10 pick.c
--- uip/pick.c  27 Apr 2006 12:00:28 -0000      1.10
+++ uip/pick.c  30 Oct 2007 04:55:01 -0000
@@ -68,6 +68,7 @@
 
 static int listsw = -1;
 
+static int putzero_done (int);
 
 int
 main (int argc, char **argv)
@@ -81,6 +82,8 @@
     struct msgs *mp;
     register FILE *fp;
 
+    done=putzero_done;
+
 #ifdef LOCALE
     setlocale(LC_ALL, "");
 #endif
@@ -287,8 +290,8 @@
 }
 
 
-int
-done (int status)
+static int
+putzero_done (int status)
 {
     if (listsw && status && !isatty (fileno (stdout)))
        printf ("0\n");
Index: uip/rcvdist.c
===================================================================
RCS file: /sources/nmh/nmh/uip/rcvdist.c,v
retrieving revision 1.10
diff -u -r1.10 rcvdist.c
--- uip/rcvdist.c       8 Mar 2006 12:14:16 -0000       1.10
+++ uip/rcvdist.c       30 Oct 2007 04:55:02 -0000
@@ -34,7 +34,7 @@
  * prototypes
  */
 static void rcvdistout (FILE *, char *, char *);
-int done (int);
+static int unlink_done (int);
 
 
 int
@@ -46,6 +46,8 @@
     char **argp, **arguments, *vec[MAXARGS];
     register FILE *fp;
 
+    done=unlink_done;
+
 #ifdef LOCALE
     setlocale(LC_ALL, "");
 #endif
@@ -268,8 +270,8 @@
 }
 
 
-int
-done (int status)
+static int
+unlink_done (int status)
 {
     if (backup[0])
        unlink (backup);
Index: uip/rcvstore.c
===================================================================
RCS file: /sources/nmh/nmh/uip/rcvstore.c,v
retrieving revision 1.11
diff -u -r1.11 rcvstore.c
--- uip/rcvstore.c      27 Apr 2006 12:00:28 -0000      1.11
+++ uip/rcvstore.c      30 Oct 2007 04:55:02 -0000
@@ -48,6 +48,7 @@
  */
 static char *tmpfilenam = NULL;
 
+static int unlink_done(int);
 
 int
 main (int argc, char **argv)
@@ -60,6 +61,8 @@
     struct msgs *mp;
     struct stat st;
 
+    done=unlink_done;
+
 #ifdef LOCALE
     setlocale(LC_ALL, "");
 #endif
@@ -228,8 +231,8 @@
 /*
  * Clean up and exit
  */
-int
-done(int status)
+static int
+unlink_done(int status)
 {
     if (tmpfilenam && *tmpfilenam)
        unlink (tmpfilenam);
Index: uip/sendsbr.c
===================================================================
RCS file: /sources/nmh/nmh/uip/sendsbr.c,v
retrieving revision 1.15
diff -u -r1.15 sendsbr.c
--- uip/sendsbr.c       21 Mar 2007 00:21:10 -0000      1.15
+++ uip/sendsbr.c       30 Oct 2007 04:55:03 -0000
@@ -41,7 +41,6 @@
 char *annotext = NULL;
 char *distfile = NULL;
 
-static int armed = 0;
 static jmp_buf env;
 
 static char    body_file_name[MAXPATHLEN + 1];         /* name of temporary 
file for body content */
@@ -56,12 +55,12 @@
  * external prototypes
  */
 int sendsbr (char **, int, char *, struct stat *, int, char *, int);
-int done (int);
 char *getusername (void);
 
 /*
  * static prototypes
  */
+static int armed_done (int);
 static void alert (char *, int);
 static int tmp_fd (void);
 static void anno (int, struct stat *);
@@ -118,7 +117,7 @@
        }
     }
 
-    armed++;
+    done=armed_done;
     switch (setjmp (env)) {
     case OK: 
        /*
@@ -154,7 +153,7 @@
        break;
     }
 
-    armed = 0;
+    done=default_done;
     if (distfile)
        unlink (distfile);
 
@@ -1075,11 +1074,10 @@
 }
 
 
-int
-done (int status)
+static int
+armed_done (int status)
 {
-    if (armed)
-       longjmp (env, status ? status : NOTOK);
+    longjmp (env, status ? status : NOTOK);
 
     exit (status);
     return 1;  /* dead code to satisfy the compiler */