Next zsh release

Next zsh release

[Peter Stephenson]

Is anything blocking the release of 4.3.12?  I think I've sorted out
everything I wanted to for the short term.

-- 
Peter Stephenson <pws@csr.com>            Software Engineer
Tel: +44 (0)1223 692070                   Cambridge Silicon Radio Limited
Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, UK


Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom

Re: Next zsh release

[Mikael Magnusson]

On 19 May 2011 15:02, Peter Stephenson <Peter.Stephenson@csr.com> wrote:
> Is anything blocking the release of 4.3.12?  I think I've sorted out
> everything I wanted to for the short term.

I have these four I want to commit, I've just been a bit lazy and not
done it yet,
PATCH: support negative LEN in ${VAR:OFFSET:LEN}
PATCH: Add g:: parameter expansion flag
PATCH: Remove some unused assignments/checks noticed by clang
"[PATCH] fix 'the the' typos" and "[PATCH] more doubles"

Let me know if you prefer them in .12 or after, or if you don't care :).

-- 
Mikael Magnusson

Re: Next zsh release

[Peter Stephenson]

On Thu, 19 May 2011 16:55:03 +0200
Mikael Magnusson <mikachu@gmail.com> wrote:
> On 19 May 2011 15:02, Peter Stephenson <Peter.Stephenson@csr.com>
> wrote:
> > Is anything blocking the release of 4.3.12?  I think I've sorted out
> > everything I wanted to for the short term.
> 
> I have these four I want to commit, I've just been a bit lazy and not
> done it yet,
> PATCH: support negative LEN in ${VAR:OFFSET:LEN}
> PATCH: Add g:: parameter expansion flag
> PATCH: Remove some unused assignments/checks noticed by clang
> "[PATCH] fix 'the the' typos" and "[PATCH] more doubles"
> 
> Let me know if you prefer them in .12 or after, or if you don't
> care :).

If you can get them in, so much the better, particularly the typo fix.

-- 
Peter Stephenson <pws@csr.com>            Software Engineer
Tel: +44 (0)1223 692070                   Cambridge Silicon Radio Limited
Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, UK




Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom

Re: Next zsh release

[Mikael Magnusson]

On 19 May 2011 17:07, Peter Stephenson <Peter.Stephenson@csr.com> wrote:
> On Thu, 19 May 2011 16:55:03 +0200
> Mikael Magnusson <mikachu@gmail.com> wrote:
>> On 19 May 2011 15:02, Peter Stephenson <Peter.Stephenson@csr.com>
>> wrote:
>> > Is anything blocking the release of 4.3.12?  I think I've sorted out
>> > everything I wanted to for the short term.
>>
>> I have these four I want to commit, I've just been a bit lazy and not
>> done it yet,
>> PATCH: support negative LEN in ${VAR:OFFSET:LEN}
>> PATCH: Add g:: parameter expansion flag
>> PATCH: Remove some unused assignments/checks noticed by clang
>> "[PATCH] fix 'the the' typos" and "[PATCH] more doubles"
>>
>> Let me know if you prefer them in .12 or after, or if you don't
>> care :).
>
> If you can get them in, so much the better, particularly the typo fix.

I've committed the first two and the last two, ie not the scary unused
assignments one. I also added a small blurb to NEWS about the first
one. Should probably have added g:: too, I can do that now if you
want. I also noticed there's no Etc/relnote_4.3.11.txt.

--- a/NEWS
+++ b/NEWS
@@ -22,6 +22,13 @@ when it should or should not exit on errors.
 The ${NAME:OFFSET:LENGTH} syntax now supports negative LENGTH, which
 counts back from the end of the string.

+The (g:opts:) flag in parameter expansion processes escape sequences like
+the echo and print builtins.  opts can be any combination of o, e and c.
+With e, acts like print rather than echo except for octal escapes which
+are controlled separately by the o option.  With c, interpret control
+sequences like "^X" as bindkey does.  Regardless of the opts, \c is not
+interpreted.
+
 Changes between versions 4.3.10 and 4.3.11
 ------------------------------------------


-- 
Mikael Magnusson

Re: Next zsh release

[Bart Schaefer]

On Thu, May 19, 2011 at 7:55 AM, Mikael Magnusson <mikachu@gmail.com> 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.

Re: Next zsh release

[Mikael Magnusson]

On 19 May 2011 17:17, Bart Schaefer <schaefer@brasslantern.com> wrote:
> On Thu, May 19, 2011 at 7:55 AM, Mikael Magnusson <mikachu@gmail.com> 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.

While trying to build statically to double check these changes, I
noticed this happens when building zftp statically:
In file included from zftp.c:54:
zftp.mdh:5:1: warning: "boot_" redefined
In file included from zftp.c:53:
tcp.mdh:5:1: warning: this is the location of the previous definition

and so on for all those functions. It seems to work though, since the
files are included in the correct order.

-- 
Mikael Magnusson

Re: Next zsh release

[Mikael Magnusson]

On 19 May 2011 17:17, Bart Schaefer <schaefer@brasslantern.com> wrote:
> On Thu, May 19, 2011 at 7:55 AM, Mikael Magnusson <mikachu@gmail.com> 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@gmail.com>
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

Re: Next zsh release

[Wayne Davison]

On Thu, May 19, 2011 at 10:53 AM, Mikael Magnusson <mikachu@gmail.com> wrote:
> Hm, in this line,
> push(setmathvar(mvp,c), mvp->lval, 0);
> isn't it undefined whether setmathvar runs first or mvp->lval is evaluated?

Yeah, it is.  That should probably be changed to this, just for safety's sake:

            c = setmathvar(mvp, c);
            push(c, mvp->lval, 0);

..wayne..

Re: Next zsh release

[Mikael Magnusson]

On 19 May 2011 20:17, Wayne Davison <wayne@thedavisons.net> wrote:
> On Thu, May 19, 2011 at 10:53 AM, Mikael Magnusson <mikachu@gmail.com> wrote:
>> Hm, in this line,
>> push(setmathvar(mvp,c), mvp->lval, 0);
>> isn't it undefined whether setmathvar runs first or mvp->lval is evaluated?
>
> Yeah, it is.  That should probably be changed to this, just for safety's sake:
>
>            c = setmathvar(mvp, c);
>            push(c, mvp->lval, 0);

make check still passes with this change, other than that I'm not sure
exactly what math construct this code actually handles. It fails if I
comment out the push() though, so it's covered I
suppose.</superparanoidmode>

--- a/Src/math.c
+++ b/Src/math.c
@@ -1161,7 +1161,8 @@ op(int what)
        }
        if (tp & (OP_E2|OP_E2IO)) {
            struct mathvalue *mvp = stack + sp + 1;
-           push(setmathvar(mvp,c), mvp->lval, 0);
+           c = setmathvar(mvp, c);
+           push(c, mvp->lval, 0);
        } else
            push(c,NULL, 0);
        return;


-- 
Mikael Magnusson