Re: [hackers] [st][PATCH] adds tmux clipboard PRIMARY support, fixed double-free

From: Omar Sandoval <osandov_AT_osandov.com>
Date: Sat, 4 Feb 2017 11:13:22 -0800

On Wed, Jan 18, 2017 at 10:34:42PM +0100, Ivan Delalande wrote:
> Hey,
>
> On Wed, Jan 18, 2017 at 05:38:20PM +0000, fpqc ?? wrote:
> > _AT_@ -2538,6 +2538,14 @@ strhandle(void)
> > break;
> > p = strescseq.args[2];
> > /* FALLTHROUGH */
>
> Watch out, you are breaking the fallthrough from case 4, you need to put
> the new case before or after the case 4/104 group.
>
> > + case 52:
> > + if (narg > 2){
> > + char *src=strescseq.args[2];
> > + size_t l = (strlen(src)/4)*3;
> > + char *buf=xmalloc(l+1);
> > + base64dec(buf, src, l);
> > + xsetsel(buf, CurrentTime);
> > + }
>
> And you probably want to exit the function here, not go to color reset,
> or "unknown str" (if you move your code after case 104).
>
> Also, try to be consistent in your spacing and declarations, and with
> the rest of the file too.
>
> > case 104: /* color reset, here p = NULL */
> > j = (narg > 1) ? atoi(strescseq.args[1]) : -1;
> > if (xsetcolorname(j, p)) {
>
>
> Looks fine otherwise, but I would still be against merging it into st,
> because of all that base64 suck.

I really like this feature, and a big part of this usefulness is being
able to copy from tmux on remote systems over SSH to my local clipboard.
But for that to work, it needs to be in the st terminfo on the remote
system, which is much easier if it was just in the official upstream
version.

Also, libresolv defines base64 helper functions. I'm not sure what sucks
less, adding that dependency or defining it ourselves :) I've attached a
version which uses libresolv.

Received on Sat Feb 04 2017 - 20:13:22 CET