Re: Next zsh release

[Mikael Magnusson <mikachu@xxxxxxxxx>]

On 19 May 2011 17:17, Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx> wrote:
> On Thu, May 19, 2011 at 7:55 AM, Mikael Magnusson <mikachu@xxxxxxxxx> wrote:
>> PATCH: Remove some unused assignments/checks noticed by clang
>
> I'm really leery of this one.  There were at least two cases someone
> spotted where the changes were wrong, weren't there?  Seems like a bad
> thing to gamble on right before a release.

Okay, I compiled everything statically and compared md5sums of all the
.o files and whatnot, and came up with this subset that doesn't result
in any changes. I have to swap the order of the printf and assignment
in compctl.c to produce the same checksum, but don't do that in the
patch.

Hm, in this line,
push(setmathvar(mvp,c), mvp->lval, 0);
isn't it undefined whether setmathvar runs first or mvp->lval is evaluated?
Point 4 on http://en.wikipedia.org/wiki/Sequence_point seems to say it is.
I am not totally sure what setmathvar() does, but it passes mvp->lval
around a bit to stuff that look like they might be modifying it.

I wrote this in an earlier mail, about modules.c:
> Should this slightly out of view "return 0;" perhaps change to "return
> ret;" instead?

After looking at this some more, I decided it shouldn't. That would
just result in it bypassing the FEAT_IGNORE thing.

---

commit a5777350ee2c177c9589500dc1b86c27dfc2be26
Author: Mikael Magnusson <mikachu@xxxxxxxxx>
Date:   Thu May 19 19:09:32 2011 +0200

    Revert "Revert "Revert parts of clang stuff that causes .o changes""

    This reverts commit 90ca5e7dd76a489f660bd8719cc8df46dc50f0a3.

diff --git a/Src/Modules/socket.c b/Src/Modules/socket.c
index 6c70d31..7369c80 100644
--- a/Src/Modules/socket.c
+++ b/Src/Modules/socket.c
@@ -56,7 +56,7 @@
 static int
 bin_zsocket(char *nam, char **args, Options ops, UNUSED(int func))
 {
-    int err=1, verbose=0, test=0, targetfd=0;
+    int verbose=0, test=0, targetfd=0;
     ZSOCKLEN_T len;
     struct sockaddr_un soun;
     int sfd;
@@ -230,7 +230,7 @@ bin_zsocket(char *nam, char **args, Options ops,
UNUSED(int func))
 	soun.sun_family = AF_UNIX;
 	strncpy(soun.sun_path, args[0], sizeof(soun.sun_path)-1);
 	
-	if ((err = connect(sfd, (struct sockaddr *)&soun, sizeof(struct
sockaddr_un)))) {
+	if (connect(sfd, (struct sockaddr *)&soun, sizeof(struct sockaddr_un))) {
 	    zwarnnam(nam, "connection failed: %e", errno);
 	    close(sfd);
 	    return 1;
diff --git a/Src/Modules/zftp.c b/Src/Modules/zftp.c
index 8d688ab..e9d1d20 100644
--- a/Src/Modules/zftp.c
+++ b/Src/Modules/zftp.c
@@ -2045,7 +2045,7 @@ zfgetinfo(char *prompt, int noecho)
     }

     if (fgets(instr, 256, stdin) == NULL)
-	instr[len = 0] = '\0';
+	instr[0] = '\0';
     else if (instr[len = strlen(instr)-1] == '\n')
 	instr[len] = '\0';

diff --git a/Src/Zle/compcore.c b/Src/Zle/compcore.c
index 5514e2e..603ef93 100644
--- a/Src/Zle/compcore.c
+++ b/Src/Zle/compcore.c
@@ -1149,7 +1149,7 @@ check_param(char *s, int set, int test)
     if (found &&
 	p[1] != Inpar && p[1] != Inbrack && p[1] != Snull) {
 	/* This is a parameter expression, not $(...), $[...], $'...'. */
-	char *b = p + 1, *e = b, *ie;
+	char *b = p + 1, *e, *ie;
 	int n = 0, br = 1, nest = 0;

 	if (*b == Inbrace) {
diff --git a/Src/Zle/compctl.c b/Src/Zle/compctl.c
index 0143370..9f176dd 100644
--- a/Src/Zle/compctl.c
+++ b/Src/Zle/compctl.c
@@ -1466,7 +1466,7 @@ printcompctl(char *s, Compctl cc, int
printflags, int ispat)
 	    c = cc2->cond;

 	    printf(" '");
-	    for (c = cc2->cond; c;) {
+	    while (c) {
 		/* loop over or's */
 		o = c->or;
 		while (c) {
diff --git a/Src/Zle/zle_keymap.c b/Src/Zle/zle_keymap.c
index a08caa0..8a8a06c 100644
--- a/Src/Zle/zle_keymap.c
+++ b/Src/Zle/zle_keymap.c
@@ -1525,7 +1525,6 @@ getkeycmd(void)

 	if (++hops == 20) {
 	    zerr("string inserting another one too many times");
-	    hops = 0;
 	    return NULL;
 	}
 	pb = unmetafy(ztrdup(str), &len);
diff --git a/Src/math.c b/Src/math.c
index a5b1925..e02a6d1 100644
--- a/Src/math.c
+++ b/Src/math.c
@@ -969,7 +969,6 @@ void
 op(int what)
 {
     mnumber a, b, c, *spval;
-    char *lv;
     int tp = type[what];

     if (errflag)
@@ -1162,7 +1161,6 @@ op(int what)
 	}
 	if (tp & (OP_E2|OP_E2IO)) {
 	    struct mathvalue *mvp = stack + sp + 1;
-	    lv = stack[sp+1].lval;
 	    push(setmathvar(mvp,c), mvp->lval, 0);
 	} else
 	    push(c,NULL, 0);
diff --git a/Src/module.c b/Src/module.c
index 219bdfa..26eb779 100644
--- a/Src/module.c
+++ b/Src/module.c
@@ -426,14 +426,13 @@ static int
 add_autobin(const char *module, const char *bnam, int flags)
 {
     Builtin bn;
-    int ret;

     bn = zshcalloc(sizeof(*bn));
     bn->node.nam = ztrdup(bnam);
     bn->optstr = ztrdup(module);
     if (flags & FEAT_AUTOALL)
 	bn->node.flags |= BINF_AUTOALL;
-    if ((ret = addbuiltin(bn))) {
+    if (addbuiltin(bn)) {
 	builtintab->freenode(&bn->node);
 	if (!(flags & FEAT_IGNORE))
 	    return 1;
diff --git a/Src/sort.c b/Src/sort.c
index 3d00bb5..e20e386 100644
--- a/Src/sort.c
+++ b/Src/sort.c
@@ -220,7 +220,7 @@ strmetasort(char **array, int sortwhat, int *unmetalenp)
     for (arrptr = array, sortptrarrptr = sortptrarr, sortarrptr = sortarr;
 	 *arrptr; arrptr++, sortptrarrptr++, sortarrptr++) {
 	char *metaptr;
-	int needlen, needalloc;
+	int needlen;
 	*sortptrarrptr = sortarrptr;
 	sortarrptr->orig = *arrptr;

@@ -251,8 +251,7 @@ strmetasort(char **array, int sortwhat, int *unmetalenp)
 	 * Either we're going to need to copy it to transform it,
 	 * or we need to unmetafy it.
 	 */
-	if ((needalloc = (sortwhat &
-			  (SORTIT_IGNORING_CASE|SORTIT_IGNORING_BACKSLASHES)))
+	if ((sortwhat & (SORTIT_IGNORING_CASE|SORTIT_IGNORING_BACKSLASHES))
 	    || *metaptr == Meta) {
 	    char *s, *t, *src = *arrptr, *dst;
 	    int len;

-- 
Mikael Magnusson