Unmetafy of getsparam()

[Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx>]

On Jul 19,  1:28am, Oliver Kiddle wrote:
}
} Bart wrote:
} > unmeta() in the getsparam() call chain, which is used to retrieve the
} > values of e.g. PROMPT_EOL_MARK, TMPPREFIX, EDITOR, HISTFILE, ...  Do
} > all of those need unmetafying too?
} 
} TMPPREFIX+=$'\u2584'
} noglob echo =(echo)
} /tmp/zsha>#a>#clEH3p
} That seems to be picking up extra garbage.
} 
} hist.c around line 2721, explicitly does open(unmeta(fn), ....
} but the not HAVE_SYMLINK branch in lockhistfile doesn't use unmeta.
} And none of the zerr calls bother.

Here's a patch that attempts to clean up [un]metafication of non-special
parameter values used internally to the shell.  Well, some of them.

I'm unsure what to do with:

- ENV in init.c -- for parsestr() is a tokenized string also metafied?
- untok_and_escape() in subst.c
- spckword() in utils.c -- patcompile expects a metafied string?
- all of hist.c is a mess with respect to handling of HISTFILE
- getcvar() in math.c
- savesession() in zftp.c
- LISTPROMPT in complistmatches()
- MENUSELECT, MENUSCROLL, MENUMODE, MENUPROMPT in domenuselect()

Specifically with respect to hist.c, it's the caller of gettempfile()
and by extension gettempname() so if metafication changes there it
may need to follow through into changes in utils.c.

Please check my work.  The unmeta() calls in argumets of setlang() et al.
are probably superfluous since it's doubtful that any valid LC_* or LANG
string is going to need metafication.


diff --git a/Src/Modules/newuser.c b/Src/Modules/newuser.c
index 71902da..efdb2ab 100644
--- a/Src/Modules/newuser.c
+++ b/Src/Modules/newuser.c
@@ -67,7 +67,7 @@ check_dotfile(const char *dotdir, const char *fname)
 int
 boot_(UNUSED(Module m))
 {
-    const char *dotdir = getsparam("ZDOTDIR");
+    const char *dotdir = getsparam_u("ZDOTDIR");
     const char *spaths[] = {
 #ifdef SITESCRIPT_DIR
 	SITESCRIPT_DIR,
diff --git a/Src/Modules/zftp.c b/Src/Modules/zftp.c
index 09d4bd7..30f5176 100644
--- a/Src/Modules/zftp.c
+++ b/Src/Modules/zftp.c
@@ -731,7 +731,7 @@ zfgetmsg(void)
     stopit = (*ptr++ != '-');
 
     queue_signals();
-    if (!(verbose = getsparam("ZFTP_VERBOSE")))
+    if (!(verbose = getsparam_u("ZFTP_VERBOSE")))
 	verbose = "";
     if (strchr(verbose, lastcodestr[0])) {
 	/* print the whole thing verbatim */
@@ -1785,7 +1785,7 @@ zftp_open(char *name, char **args, int flags)
 	char *hname;
 	alarm(0);
 	queue_signals();
-	if ((hname = getsparam("ZFTP_HOST")) && *hname) 
+	if ((hname = getsparam_u("ZFTP_HOST")) && *hname) 
 	    zwarnnam(name, "timeout connecting to %s", hname);
 	else
 	    zwarnnam(name, "timeout on host name lookup");
@@ -3077,7 +3077,7 @@ bin_zftp(char *name, char **args, UNUSED(Options ops), UNUSED(int func))
     }
 
     queue_signals();
-    if ((prefs = getsparam("ZFTP_PREFS"))) {
+    if ((prefs = getsparam_u("ZFTP_PREFS"))) {
 	zfprefs = 0;
 	for (ptr = prefs; *ptr; ptr++) {
 	    switch (toupper(STOUC(*ptr))) {
diff --git a/Src/Zle/complist.c b/Src/Zle/complist.c
index f37a432..fd90ccb 100644
--- a/Src/Zle/complist.c
+++ b/Src/Zle/complist.c
@@ -507,8 +507,8 @@ getcols()
     max_caplen = lr_caplen = 0;
     mcolors.flags = 0;
     queue_signals();
-    if (!(s = getsparam("ZLS_COLORS")) &&
-	!(s = getsparam("ZLS_COLOURS"))) {
+    if (!(s = getsparam_u("ZLS_COLORS")) &&
+	!(s = getsparam_u("ZLS_COLOURS"))) {
 	for (i = 0; i < NUM_COLS; i++)
 	    mcolors.files[i] = filecol("");
 	mcolors.pats = NULL;
diff --git a/Src/Zle/zle_misc.c b/Src/Zle/zle_misc.c
index d350688..556ce5b 100644
--- a/Src/Zle/zle_misc.c
+++ b/Src/Zle/zle_misc.c
@@ -1552,13 +1552,13 @@ makesuffix(int n)
 {
     char *suffixchars;
 
-    if (!(suffixchars = getsparam("ZLE_REMOVE_SUFFIX_CHARS")))
+    if (!(suffixchars = getsparam_u("ZLE_REMOVE_SUFFIX_CHARS")))
 	suffixchars = " \t\n;&|";
 
     addsuffixstring(SUFTYP_POSSTR, 0, suffixchars, n);
 
     /* Do this second so it takes precedence */
-    if ((suffixchars = getsparam("ZLE_SPACE_SUFFIX_CHARS")) && *suffixchars)
+    if ((suffixchars = getsparam_u("ZLE_SPACE_SUFFIX_CHARS")) && *suffixchars)
 	addsuffixstring(SUFTYP_POSSTR, SUFFLAGS_SPACE, suffixchars, n);
 
     suffixlen = n;
diff --git a/Src/init.c b/Src/init.c
index 0fe4d75..2ef9099 100644
--- a/Src/init.c
+++ b/Src/init.c
@@ -1426,7 +1426,7 @@ sourcehome(char *s)
     char *h;
 
     queue_signals();
-    if (EMULATION(EMULATE_SH|EMULATE_KSH) || !(h = getsparam("ZDOTDIR"))) {
+    if (EMULATION(EMULATE_SH|EMULATE_KSH) || !(h = getsparam_u("ZDOTDIR"))) {
 	h = home;
 	if (!h)
 	    return;
diff --git a/Src/params.c b/Src/params.c
index 7d0c852..efbe92b 100644
--- a/Src/params.c
+++ b/Src/params.c
@@ -2639,6 +2639,15 @@ getsparam(char *s)
     return getstrvalue(v);
 }
 
+/**/
+mod_export char *
+getsparam_u(char *s)
+{
+    if ((s = getsparam(s)))
+	return unmetafy(s, NULL);
+    return s;
+}
+
 /* Retrieve an array parameter */
 
 /**/
@@ -3971,7 +3980,7 @@ setlang(char *x)
     struct localename *ln;
     char *x2;
 
-    if ((x2 = getsparam("LC_ALL")) && *x2)
+    if ((x2 = getsparam_u("LC_ALL")) && *x2)
 	return;
 
     /*
@@ -3985,10 +3994,10 @@ setlang(char *x)
      * from this is meaningless.  So just all $LANG to show through in
      * that case.
      */
-    setlocale(LC_ALL, x ? x : "");
+    setlocale(LC_ALL, x ? unmeta(x) : "");
     queue_signals();
     for (ln = lc_names; ln->name; ln++)
-	if ((x = getsparam(ln->name)) && *x)
+	if ((x = getsparam_u(ln->name)) && *x)
 	    setlocale(ln->category, x);
     unqueue_signals();
 }
@@ -4004,7 +4013,7 @@ lc_allsetfn(Param pm, char *x)
      * that with any LC_* that are set.
      */
     if (!x || !*x) {
-	x = getsparam("LANG");
+	x = getsparam_u("LANG");
 	if (x && *x) {
 	    queue_signals();
 	    setlang(x);
@@ -4012,7 +4021,7 @@ lc_allsetfn(Param pm, char *x)
 	}
     }
     else
-	setlocale(LC_ALL, x);
+	setlocale(LC_ALL, unmeta(x));
 }
 
 /**/
@@ -4020,7 +4029,7 @@ void
 langsetfn(Param pm, char *x)
 {
     strsetfn(pm, x);
-    setlang(x);
+    setlang(unmeta(x));
 }
 
 /**/
@@ -4046,7 +4055,7 @@ lcsetfn(Param pm, char *x)
     if (x && *x) {
 	for (ln = lc_names; ln->name; ln++)
 	    if (!strcmp(ln->name, pm->node.nam))
-		setlocale(ln->category, x);
+		setlocale(ln->category, unmeta(x));
     }
     unqueue_signals();
 }
diff --git a/Src/utils.c b/Src/utils.c
index 8ff575f..ba90564 100644
--- a/Src/utils.c
+++ b/Src/utils.c
@@ -248,7 +248,7 @@ VA_DCL
 
     VA_START(ap, message);
     VA_GET_ARG(ap, message, const char *);
-    if ((filename = getsparam("ZSH_DEBUG_LOG")) != NULL &&
+    if ((filename = getsparam_u("ZSH_DEBUG_LOG")) != NULL &&
 	(file = fopen(filename, "a")) != NULL) {
 	zerrmsg(file, message, ap);
 	fclose(file);
@@ -1949,7 +1949,8 @@ extern char *_mktemp(char *);
 /* Get a unique filename for use as a temporary file.  If "prefix" is
  * NULL, the name is relative to $TMPPREFIX; If it is non-NULL, the
  * unique suffix includes a prefixed '.' for improved readability.  If
- * "use_heap" is true, we allocate the returned name on the heap. */
+ * "use_heap" is true, we allocate the returned name on the heap.
+ * The string passed as "prefix" is expected to be metafied. */
 
 /**/
 mod_export char *
@@ -1976,6 +1977,9 @@ gettempname(const char *prefix, int use_heap)
     return ret;
 }
 
+/* The gettempfile() "prefix" is expected to be metafied, see hist.c
+ * and gettempname(). */
+
 /**/
 mod_export int
 gettempfile(const char *prefix, int use_heap, char **tempname)
@@ -3585,7 +3589,7 @@ zbeep(void)
 {
     char *vb;
     queue_signals();
-    if ((vb = getsparam("ZBEEP"))) {
+    if ((vb = getsparam_u("ZBEEP"))) {
 	int len;
 	vb = getkeystring(vb, &len, GETKEYS_BINDKEY, NULL);
 	write_loop(SHTTY, vb, len);
diff --git a/Src/watch.c b/Src/watch.c
index fe409f9..e1bdaa4 100644
--- a/Src/watch.c
+++ b/Src/watch.c
@@ -566,7 +566,7 @@ dowatch(void)
 	return;
     }
     queue_signals();
-    if (!(fmt = getsparam("WATCHFMT")))
+    if (!(fmt = getsparam_u("WATCHFMT")))
 	fmt = DEFAULT_WATCHFMT;
     while ((uct || wct) && !errflag)
 	if (!uct || (wct && ucmp(uptr, wptr) > 0))