Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/bin/sh Revamp aliases - as dumb an idea as they are, if we'r...



details:   https://anonhg.NetBSD.org/src/rev/b29ea8fe820d
branches:  trunk
changeset: 446376:b29ea8fe820d
user:      kre <kre%NetBSD.org@localhost>
date:      Mon Dec 03 06:40:26 2018 +0000

description:
Revamp aliases - as dumb an idea as they are, if we're going
to have them, they should work as documented, not cause core
dumps, reference after free, incorrect replacements, failing
to implement alias after alias, ...

The big comment that ended:
          This is a good idea ------- ***NOT***
and the hack it was describing are gone.

Note that most of this was from original CVS version 1.1
code (ie: came from the original import, even before 4.4-Lite
was merged.   That is, May 1994.  And no-one in 24.5 years
noticed (or at least complained about)  all the bugs (or at
least, most of them)).

With these changes, aliases ought to work (if you can call it
that) as they are expected to by POSIX.   Now if only we could
get POSIX to delete them (or make them optional)...

Changes partly inspired by similar changes made by FreeBSD,
(as was the previous change to alias.c, forgot ack in commit
log for that one, apologies) but done a little differently,
and perhaps with a slightly better outcome.

diffstat:

 bin/sh/alias.c  |  155 ++++++++++++++++++++++++++++++-------------------------
 bin/sh/alias.h  |    4 +-
 bin/sh/eval.c   |   26 ++-------
 bin/sh/input.c  |   21 +++++-
 bin/sh/parser.c |  122 ++++++++++++++++++++++++-------------------
 bin/sh/parser.h |   13 +++-
 bin/sh/syntax.c |   16 ++--
 bin/sh/syntax.h |    8 +-
 8 files changed, 199 insertions(+), 166 deletions(-)

diffs (truncated from 851 to 300 lines):

diff -r c41d873be6b6 -r b29ea8fe820d bin/sh/alias.c
--- a/bin/sh/alias.c    Mon Dec 03 05:22:03 2018 +0000
+++ b/bin/sh/alias.c    Mon Dec 03 06:40:26 2018 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: alias.c,v 1.19 2018/12/02 10:27:58 kre Exp $   */
+/*     $NetBSD: alias.c,v 1.20 2018/12/03 06:40:26 kre Exp $   */
 
 /*-
  * Copyright (c) 1993
@@ -37,7 +37,7 @@
 #if 0
 static char sccsid[] = "@(#)alias.c    8.3 (Berkeley) 5/4/95";
 #else
-__RCSID("$NetBSD: alias.c,v 1.19 2018/12/02 10:27:58 kre Exp $");
+__RCSID("$NetBSD: alias.c,v 1.20 2018/12/03 06:40:26 kre Exp $");
 #endif
 #endif /* not lint */
 
@@ -61,7 +61,9 @@
 STATIC int by_name(const void *, const void *);
 STATIC void list_aliases(void);
 STATIC int unalias(char *);
+STATIC struct alias **freealias(struct alias **, int);
 STATIC struct alias **hashalias(const char *);
+STATIC size_t countaliases(void);
 
 STATIC
 void
@@ -76,97 +78,80 @@
        ap = ckmalloc(sizeof (struct alias));
        ap->name = savestr(name);
        ap->flag = 0;
-       /*
-        * XXX - HACK: in order that the parser will not finish reading the
-        * alias value off the input before processing the next alias, we
-        * dummy up an extra space at the end of the alias.  This is a crock
-        * and should be re-thought.  The idea (if you feel inclined to help)
-        * is to avoid alias recursions.  The mechanism used is: when
-        * expanding an alias, the value of the alias is pushed back on the
-        * input as a string and a pointer to the alias is stored with the
-        * string.  The alias is marked as being in use.  When the input
-        * routine finishes reading the string, it markes the alias not
-        * in use.  The problem is synchronization with the parser.  Since
-        * it reads ahead, the alias is marked not in use before the
-        * resulting token(s) is next checked for further alias sub.  The
-        * H A C K is that we add a little fluff after the alias value
-        * so that the string will not be exhausted.  This is a good
-        * idea ------- ***NOT***
-        */
-#ifdef notyet
        ap->val = savestr(val);
-#else /* hack */
-       {
-       int len = strlen(val);
-       ap->val = ckmalloc(len + 2);
-       memcpy(ap->val, val, len);
-       ap->val[len] = ' ';     /* fluff */
-       ap->val[len+1] = '\0';
-       }
-#endif
        ap->next = *app;
        *app = ap;
        INTON;
 }
 
+STATIC struct alias **
+freealias(struct alias **app, int force)
+{
+       struct alias *ap = *app;
+
+       if (ap == NULL)
+               return app;
+
+       /*
+        * if the alias is currently in use (i.e. its
+        * buffer is being used by the input routine) we
+        * just null out the name instead of discarding it.
+        * If we encounter it later, when it is idle,
+        * we will finish freeing it then.
+        *
+        * Unless we want to simply free everything (INIT)
+        */
+       if (ap->flag & ALIASINUSE && !force) {
+               *ap->name = '\0';
+               return &ap->next;
+       }
+
+       INTOFF;
+       *app = ap->next;
+       ckfree(ap->name);
+       ckfree(ap->val);
+       ckfree(ap);
+       INTON;
+
+       return app;
+}
+
 STATIC int
 unalias(char *name)
 {
        struct alias *ap, **app;
 
        app = hashalias(name);
-
-       for (ap = *app; ap; app = &(ap->next), ap = ap->next) {
+       while ((ap = *app) != NULL) {
                if (equal(name, ap->name)) {
-                       /*
-                        * if the alias is currently in use (i.e. its
-                        * buffer is being used by the input routine) we
-                        * just null out the name instead of freeing it.
-                        * We could clear it out later, but this situation
-                        * is so rare that it hardly seems worth it.
-                        */
-                       if (ap->flag & ALIASINUSE)
-                               *ap->name = '\0';
-                       else {
-                               INTOFF;
-                               *app = ap->next;
-                               ckfree(ap->name);
-                               ckfree(ap->val);
-                               ckfree(ap);
-                               INTON;
-                       }
+                       (void) freealias(app, 0);
                        return 0;
                }
+               app = &ap->next;
        }
 
        return 1;
 }
 
 #ifdef mkinit
-MKINIT void rmaliases(void);
+MKINIT void rmaliases(int);
 
 SHELLPROC {
-       rmaliases();
+       rmaliases(1);
 }
 #endif
 
 void
-rmaliases(void)
+rmaliases(int force)
 {
-       struct alias *ap, *tmp;
+       struct alias **app;
        int i;
 
        INTOFF;
        for (i = 0; i < ATABSIZE; i++) {
-               ap = atab[i];
-               atab[i] = NULL;
-               while (ap) {
-                       ckfree(ap->name);
-                       ckfree(ap->val);
-                       tmp = ap;
-                       ap = ap->next;
-                       ckfree(tmp);
-               }
+               app = &atab[i];
+               while (*app)
+                       app = freealias(app, force);
        }
        INTON;
 }
@@ -176,12 +161,13 @@
 {
        struct alias *ap = *hashalias(name);
 
-       for (; ap; ap = ap->next) {
+       while (ap != NULL) {
                if (equal(name, ap->name)) {
                        if (check && (ap->flag & ALIASINUSE))
                                return NULL;
                        return ap;
                }
+               ap = ap->next;
        }
 
        return NULL;
@@ -214,12 +200,8 @@
        const struct alias **aliases;
        const struct alias *ap;
 
-       n = 0;
-       for (i = 0; i < ATABSIZE; i++)
-               for (ap = atab[i]; ap != NULL; ap = ap->next)
-                       if (ap->name[0] != '\0')
-                               n++;
-
+       INTOFF;
+       n = countaliases();
        aliases = ckmalloc(n * sizeof aliases[0]);
 
        j = 0;
@@ -227,6 +209,9 @@
                for (ap = atab[i]; ap != NULL; ap = ap->next)
                        if (ap->name[0] != '\0')
                                aliases[j++] = ap;
+       if (j != n)
+               error("Alias count botch");
+       INTON;
 
        qsort(aliases, n, sizeof aliases[0], by_name);
 
@@ -239,6 +224,34 @@
        ckfree(aliases);
 }
 
+/*
+ * Count how many aliases are defined (skipping any
+ * that have been deleted, but don't know it yet).
+ * Use this opportunity to clean up any of those
+ * zombies that are no longer needed.
+ */
+STATIC size_t
+countaliases(void)
+{
+       struct alias *ap, **app;
+       size_t n;
+       int i;
+
+       n = 0;
+       for (i = 0; i < ATABSIZE; i++)
+               for (app = &atab[i]; (ap = *app) != NULL;) {
+                       if (ap->name[0] != '\0')
+                               n++;
+                       else {
+                               app = freealias(app, 0);
+                               continue;
+                       }
+                       app = &ap->next;
+               }
+
+       return n;
+}
+
 int
 aliascmd(int argc, char **argv)
 {
@@ -277,12 +290,14 @@
 
        while ((i = nextopt("a")) != '\0') {
                if (i == 'a') {
-                       rmaliases();
+                       rmaliases(0);
                        return 0;
                }
        }
+
+       (void)countaliases();   /* delete any dead ones */
        for (i = 0; *argptr; argptr++)
-               i = unalias(*argptr);
+               i |= unalias(*argptr);
 
        return i;
 }
diff -r c41d873be6b6 -r b29ea8fe820d bin/sh/alias.h
--- a/bin/sh/alias.h    Mon Dec 03 05:22:03 2018 +0000
+++ b/bin/sh/alias.h    Mon Dec 03 06:40:26 2018 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: alias.h,v 1.8 2014/06/18 18:17:30 christos Exp $       */
+/*     $NetBSD: alias.h,v 1.9 2018/12/03 06:40:26 kre Exp $    */
 
 /*-
  * Copyright (c) 1993
@@ -45,4 +45,4 @@
 
 struct alias *lookupalias(const char *, int);
 const char *alias_text(void *, const char *);
-void rmaliases(void);
+void rmaliases(int);
diff -r c41d873be6b6 -r b29ea8fe820d bin/sh/eval.c
--- a/bin/sh/eval.c     Mon Dec 03 05:22:03 2018 +0000
+++ b/bin/sh/eval.c     Mon Dec 03 06:40:26 2018 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: eval.c,v 1.165 2018/11/30 23:22:45 kre Exp $   */
+/*     $NetBSD: eval.c,v 1.166 2018/12/03 06:40:26 kre Exp $   */
 
 /*-
  * Copyright (c) 1993
@@ -37,7 +37,7 @@
 #if 0
 static char sccsid[] = "@(#)eval.c     8.9 (Berkeley) 6/8/95";
 #else
-__RCSID("$NetBSD: eval.c,v 1.165 2018/11/30 23:22:45 kre Exp $");
+__RCSID("$NetBSD: eval.c,v 1.166 2018/12/03 06:40:26 kre Exp $");
 #endif
 #endif /* not lint */
 
@@ -888,18 +888,11 @@
        varflag = 1;
        /* Expand arguments, ignoring the initial 'name=value' ones */
        for (argp = cmd->ncmd.args ; argp ; argp = argp->narg.next) {
-               char *p = argp->narg.text;
-
+               if (varflag && isassignment(argp->narg.text))



Home | Main Index | Thread Index | Old Index