Re: PATCH: Remove some unused assignments/checks noticed by clang

[Mikael Magnusson <mikachu@xxxxxxxxx>]

On 14 May 2011 19:22, Wayne Davison <wayned@xxxxxxxxxxxxxxxxxxxxx> wrote:
> On Fri, May 13, 2011 at 5:23 PM, Mikael Magnusson <mikachu@xxxxxxxxx> wrote:
>> --- a/Src/Zle/compcore.c
>> +++ b/Src/Zle/compcore.c
>> @@ -1162,7 +1162,6 @@ check_param(char *s, int set, int test)
>>        if (*b == '#' || *b == Pound || *b == '+')
>>            b++;
>>
>> -       e = b;
>>        if (br) {
>>            while (*e == (test ? Dnull : '"'))
>>                e++, parq++;
>
> That change looks wrong to me.  You should remove the " = b"
> initializer in the declaration:
>
>        char *b = p + 1, *e = b, *ie;
>
> The later assignment could occur after b changes value, and thus
> removing it means that e starts further back in the string.

Oops, but also weird. With the change, the md5sum of Src/zsh remains
the same, but Src/Zle/compcore..o changes.

> The same comment applies to the similar change in zle_tricky.c.
>
>> --- a/Src/Zle/computil.c
>> +++ b/Src/Zle/computil.c
>> @@ -4465,7 +4465,7 @@ cfp_opt_pats(char **pats, char *matcher)
>>            q = dupstring(q);
>>            t = q + strlen(q) - 1;
>>            if (*t == ')') {
>> -               for (s = t--; t > q; t--)
>> +               while (--t)
>>                    if (*t == ')' || *t == '|' || *t == '~' || *t == '(')
>>                        break;
>>                if (t != q && *t == '(')
>
> That while should be while (--t > q), since indexing a string back to
> 0 is not a good idea.

Indeed. I'd better look at the whole patch once more. :)

-- 
Mikael Magnusson