Re: segfault in 'ls' completion

[Vin Shelton <acs@alumni.princeton.edu>]

[-- Attachment #1: Type: text/plain, Size: 3143 bytes --]

Stops the segfault, and generates a conforming list.

Thanks,
  Vin

On Wed, Dec 15, 2021 at 6:57 PM Oliver Kiddle <opk@zsh.org> wrote:

> 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? */
>

[-- Attachment #2: Type: text/html, Size: 4349 bytes --]