Re: segfault in 'ls' completion

[Oliver Kiddle <opk@xxxxxxx>]

Bart Schaefer wrote:
>     I was able to reproduce this

I couldn't initially but as you could, I thought I'd better try again.

> reverted to revision e40938c128 (before the workers/49499 changes to
> computil.c) and was no longer able to reproduce in that version, but that does
> also revert some changes to _arguments.

It actually seems it was 49518 / 7cb980b which was only applied
yesterday having been posted in October and forgotten. I had a nagging
suspicion that I needed to further check over that. My mistake was
mixing up hex and decimal when looking at the ASCII table to work out
how to rearrange the single character option letters within the lookup
array. 20 should have been 0x20 or 32.

'y' appears before the tab and the word starts with something that isn't
'-'. So it uses the + options offset which are later and as y is within
the difference between decimal and hex 20 from the end of the characters
this caused it index beyond the end of the array.

Following this, I also wondered what it's doing strcmping '/usr/libpy'
against every possible ls option. That's nothing new. Note that
_arguments only lets you start options with - or + and we check for
those explicitly in a few places. I think it's worth optimising this
away. The check could perhaps be factored into ca_get_opt() and
ca_get_sopt() ?

If someone has a moment, please check that the calculation in
single_index() makes sense. The array is allocated as
  ret->single = (Caopt *) zalloc(188 * sizeof(Caopt));

Oliver

diff --git a/Src/Zle/computil.c b/Src/Zle/computil.c
index c49d688c8..59abb4cc4 100644
--- a/Src/Zle/computil.c
+++ b/Src/Zle/computil.c
@@ -1088,10 +1088,10 @@ bslashcolon(char *s)
 static int
 single_index(char pre, char opt)
 {
-    if (opt <= 20 || opt > 0x7e)
+    if (opt <= 0x20 || opt > 0x7e)
 	return -1;
 
-    return opt + (pre == '-' ? -21 : 94 - 21);
+    return opt + (pre == '-' ? -0x21 : 94 - 0x21);
 }
 
 /* Parse an argument definition. */
@@ -2158,7 +2158,8 @@ ca_parse_line(Cadef d, Cadef all, int multi, int first)
 
 	/* See if it's an option. */
 
-	if (state.opt == 2 && (state.curopt = ca_get_opt(d, line, 0, &pe)) &&
+	if (state.opt == 2 && (*line == '-' || *line == '+') &&
+	    (state.curopt = ca_get_opt(d, line, 0, &pe)) &&
 	    (state.curopt->type == CAO_OEQUAL ?
 	     (compwords[cur] || pe[-1] == '=') :
 	     (state.curopt->type == CAO_EQUAL ?
@@ -2206,6 +2207,7 @@ ca_parse_line(Cadef d, Cadef all, int multi, int first)
 		state.curopt = NULL;
 	    }
 	} else if (state.opt == 2 && d->single &&
+		   (*line == '-' || *line == '+') &&
 		   ((state.curopt = ca_get_sopt(d, line, &pe, &sopts)) ||
 		    (cur != compcurrent && sopts && nonempty(sopts)))) {
 	    /* Or maybe it's a single-letter option? */