On Sat, Mar 17, 2018 at 08:29:27PM +0100, Daniel Tameling wrote:
>
> Hiltjo Posthuma writes:
>
> > On Sat, Mar 17, 2018 at 11:04:22AM +0100, Daniel Tameling wrote:
> >>
> >> Hi,
> >>
> >> the clipboard patch st-clipboard-20180309-c5ba9c0.diff can lead to
> >> double frees with st 0.8. This is because, starting with commit
> >> cfc7acdfd923924ae150a32061fb95987697b159, in brelease every time Button1
> >> is released mousesel(e, 1) is called. Up to the commit, there was some
> >> code that only called mousel when an actual selection was made. Now if
> >> you just left-click setsel is called in mousesel with str being NULL
> >> because that is what getsel() returns. This means that xsel.primary gets
> >> set to NULL. If you now add xclipcopy() to setsel according to the
> >> patch, the following can happen:
> >>
> >> 1. You make make a selection -> clipcopy copies it to the clipboard.
> >> 2. You left-click -> clipcopy frees xsel.clipcopy, but because
> >> xsel.primary was set to NULL in setsel, it doesn't do anything else
> >> 3. You left click again -> clipcopy tries to free xsel.clipcopy again,
> >> and st crashes
> >>
> >> Also note that the mentioned commit changed the behaviour of the
> >> primary. Before it, the primary survived up until you made a new
> >> selection. Now it gets erased if you left-click once.
> >>
> >> --
> >> Kind regards,
> >> Daniel
> >>
> >
> > OK, so where is the patch?
>
> Thanks, Hiltjo. Your commit earlier today took care of the crashes.
> I think the easiest way to restore the old behaviour that the primary
> doesn't get deleted by a simple left click is to return early in setsel
> if str is NULL. But I am not sure whether that is the right behaviour
> anyway or whether there is a better way to achieve it.
>
> --
> Best regards,
> Daniel
>
>
> diff --git a/x.c b/x.c
> index 12bc86b..a68399b 100644
> --- a/x.c
> +++ b/x.c
> _AT_@ -618,6 +618,8 @@ selrequest(XEvent *e)
> void
> setsel(char *str, Time t)
> {
> + if (!str)
> + return;
> free(xsel.primary);
> xsel.primary = str;
>
Thanks for the patch. After a brief test this seems to fix the selection
issue.
Can more people test this and check for a potential regression?
--
Kind regards,
Hiltjo
Received on Sun Mar 18 2018 - 18:59:54 CET