Re: [PATCH] allocate origline by ztrdup(), not by dupstring()

[Oliver Kiddle <okiddle@xxxxxxxxxxx>]

On 7 Jan, "Jun T." wrote:
> Currently, domenuselect() allocates origline in heap by dupstring(),
> but if menuselect() is called directly as a widget then the heap has
> already been freed (and may have been re-used for other variables).

I noticed problems which I traced back to this change.

> diff --git a/Src/Zle/complist.c b/Src/Zle/complist.c
> @@ -2882,7 +2882,8 @@ domenuselect(Hookdef dummy, Chdata dat)
> -	    origline = u->origline;
> +	    zsfree(origline);
> +	    origline = ztrdup(u->origline);

This part is run when pressing backspace, it seems to always be the case
that origline == u->origline before this block runs. So it is copying freed
memory. I tried the obvious change of checking that condition first but
that seems to result in deleted characters being treated as if they were
part of the original string before completion. It is also calling zsfree
on origline before ther very first time that it is initialised.

I think it might be easier to approach this by revisiting the original
problem. I've tried binding a widget direct to menuselect but haven't
been able to get anything odd to happen.

origline is initialised in docomplete. pushheap() is done later in
do_completion(). That would seem not to be a problem for a later
menu-select widget accessing origline. I am, however, suspicious of the
line which does origline = modeline because the value of modeline does,
I think, come from dupstring() while completion is active. Setting a
breakpoint at that line, I've only managed to trigger it when origline
and modeline are already equal (in strcmp terms) so I'm not sure of the
situation that code is there to handle. Any ideas?

Oliver