Re: [hackers] [sbase] Revert "ed: remove double free in join()" || Roberto E. Vargas Caballero

[Thomas Mannay]

Hello Roberto,

The trouble with reverting my commit is that readding the double free completely
crashes ed if more than one join is performed. I think this patch (which also 
reverts
back to having no double free) should handle your concern via blocking signal 
handling
until the join is finished.

Thomas

--
Thomas Mannay 


0001-ed-ignore-signals-inside-of-join.patch
Description: Binary data

Re: [hackers] [dwm] [PATCH 2/3] Button passthrough when client is not focused

[Silvan Jegen]

Hi Markus

>From what I understand, in case that the client does not have focus
already, with this patch we call XGrabButton twice compared to before the
patch. I assume that corresponds to the advertised functionality.

One more comment below.

On Sat, Jan 07, 2017 at 05:21:29PM +0100, Markus Teich wrote:
> Before this change it is not possible to press a button in a client on the 
> first
> click if the client is not yet focused. The first click on the button would
> only focus the client and a second click on the button is needed to activate 
> it.
> This situation can occur when moving the mouse over a client (therefore 
> focusing
> it) and then moving the focus to another client with keyboard shortcuts.
> 
> After this commit the behavior is fixed and button presses on unfocused 
> clients
> are passed to the client correctly.
> ---
> 
> 
> Heyho,
> 
> this patch is the same as submitted previously. I just added it to this patch
> series for a better overview of the pending patches.
> 
> --Markus
> 
> 
>  dwm.c | 21 +++--
>  1 file changed, 11 insertions(+), 10 deletions(-)
> 
> diff --git a/dwm.c b/dwm.c
> index 3f80b63..9c01d1a 100644
> --- a/dwm.c
> +++ b/dwm.c
> @@ -446,6 +446,8 @@ buttonpress(XEvent *e)
>   click = ClkWinTitle;
>   } else if ((c = wintoclient(ev->window))) {
>   focus(c);
> + restack(selmon);
> + XAllowEvents(dpy, ReplayPointer, CurrentTime);
>   click = ClkClientWin;
>   }
>   for (i = 0; i < LENGTH(buttons); i++)
> @@ -932,17 +934,16 @@ grabbuttons(Client *c, int focused)
>   unsigned int i, j;
>   unsigned int modifiers[] = { 0, LockMask, numlockmask, 
> numlockmask|LockMask };
>   XUngrabButton(dpy, AnyButton, AnyModifier, c->win);
> - if (focused) {
> - for (i = 0; i < LENGTH(buttons); i++)
> - if (buttons[i].click == ClkClientWin)
> - for (j = 0; j < LENGTH(modifiers); j++)
> - XGrabButton(dpy, 
> buttons[i].button,
> - buttons[i].mask | 
> modifiers[j],
> - c->win, False, 
> BUTTONMASK,
> - GrabModeAsync, 
> GrabModeSync, None, None);
> - } else
> + if (!focused)
>   XGrabButton(dpy, AnyButton, AnyModifier, c->win, False,
> - BUTTONMASK, GrabModeAsync, GrabModeSync, 
> None, None);
> + BUTTONMASK, GrabModeSync, GrabModeSync, 
> None, None);

Here we pass "GrabModeSync" instead of "GrabModeAsync" as the "pointer
mode". According to https://tronche.com/gui/x/xlib/input/XGrabPointer.html
if the mode is "GrabModeSync",

"the state of the pointer, as seen by client applications, appears to
freeze, and the X server generates no further pointer events until
the grabbing client calls XAllowEvents() or until the pointer grab
is released."

Since we are calling XAllowEvents on buttonpress that should make sure
that only the focused client gets any pointer events, I assume?

I have been runnig with this patch for most of the day and haven't
encountered any issues.


Cheers,

Silvan


> + for (i = 0; i < LENGTH(buttons); i++)
> + if (buttons[i].click == ClkClientWin)
> + for (j = 0; j < LENGTH(modifiers); j++)
> + XGrabButton(dpy, buttons[i].button,
> + buttons[i].mask | 
> modifiers[j],
> + c->win, False, BUTTONMASK,
> + GrabModeAsync, 
> GrabModeSync, None, None);
>   }
>  }
>  
> -- 
> 2.10.2
> 
> 

Re: [hackers] [PATCH] simplify client moving on monitor count decrease

[Silvan Jegen]

Hi Markus

On Wed, Jan 04, 2017 at 06:05:33PM +0100, Markus Teich wrote:
> ---
>  dwm.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)

Looks good to me.

I have tested this patch yesterday and today in daily use and haven't
found any issues on my 2-monitor setup (yet).


Cheers,

Silvan

 
> diff --git a/dwm.c b/dwm.c
> index 4eba952..88d49d7 100644
> --- a/dwm.c
> +++ b/dwm.c
> @@ -1896,9 +1896,8 @@ updategeom(void)
>   /* less monitors available nn < n */
>   for (i = nn; i < n; i++) {
>   for (m = mons; m && m->next; m = m->next);
> - while (m->clients) {
> + while ((c = m->clients)) {
>   dirty = 1;
> - c = m->clients;
>   m->clients = c->next;
>   detachstack(c);
>   c->mon = mons;
> -- 
> 2.10.2
> 
> 

Re: [hackers] [sbase] [PATCH 1/5] Remove st != NULL checks from recursor functions

[Silvan Jegen]

Hi Laslo

On Tue, Dec 27, 2016 at 02:59:56PM +0100, Laslo Hunhold wrote:
> On Wed, 14 Dec 2016 19:40:02 -0800
> Michael Forney  wrote:
> 
> Hey Michael,
> 
> > In the description of 3111908b034c73673a2f079b2b13a88c18379baa, it
> > says that the functions must be able to handle st being NULL, but
> > recurse always passes a valid pointer. The only function that was
> > ever passed NULL was rm(), but this was changed to go through recurse
> > in 2f4ab527391135e651b256f8654b050ea4a48f3d, so now the checks are
> > pointless.
> 
> have you tested this patchset extensively? I hate to admit that the
> recursor-subsystem is probably the most fragile part of sbase and
> really need more feedback on these patches by more people (Silvan, have
> you had the chance to test this?).

Sadly, I have not :/

I just did some very basic testing using the following directory
structure.

testdir/
testdir/testdir2
testdir/testdir2/testB.txt
testdir/testdir2/testA.txt
testdir/link (symbolic link)
testdir/test1.txt
testdir/test2.txt

I used the recursive options of tar, chgrp, chmod, chown, du and rm after
applying Michael's patch on these files/directories and everything worked
as expected.

Ideally, somebody that uses sbase in a development system regularly
should apply and test the patches further.


Cheers,

Silvan
 
> Cheers
> 
> Laslo
> 
> -- 
> Laslo Hunhold 
> 

Re: [hackers] [sbase] ed: Don't use strlcpy() || Roberto E. Vargas Caballero

[Laslo Hunhold]

On Tue, 10 Jan 2017 08:56:46 +0100 (CET)
g...@suckless.org wrote:

Hey Roberto,

> All the buffers related to files have FILENAME_MAX size, so it is
> impossible to have any buffer overrun.

I consider this way of thinking harmful, because it involves
assumptions about the code that are met in a different location. In
case FILENAME_MAX is changed to some strange value, this entire
building of thought breaks together.
There's no reason not to use strlcpy, as it saves you from buffer
overruns and other things. Nobody can possibly guarantee that some evil
input has a non-null-terminated fname and we write to savfname without
bounds.

As Dimitris likes to say, programs spend 99% with I/O, so this
"optimization" here won't make a difference. Premature optimization is
the root of all evil, and given we get strlcpy() for free from libutil,
I strongly suggest we keep the usage here.

Cheers

Laslo

-- 
Laslo Hunhold 

[hackers] [sbase] ed: fix commit 2ccc1e8 || Roberto E. Vargas Caballero

[git]

commit 9ab1478f1eb6a9a355d394af3c0cfa69850245fe
Author: Roberto E. Vargas Caballero 
AuthorDate: Tue Jan 10 11:28:58 2017 +0100
Commit: Roberto E. Vargas Caballero 
CommitDate: Tue Jan 10 11:30:34 2017 +0100

ed: fix commit 2ccc1e8

The patch was wrong because the prototype of
strcpy is different to the prototype of strlcpy

diff --git a/ed.c b/ed.c
index 82fb784..4b28848 100644
--- a/ed.c
+++ b/ed.c
@@ -611,8 +611,7 @@ dowrite(const char *fname, int trunc)
curln = line2;
if (fclose(fp))
error("input/output error");
-   if (strcpy(savfname, fname, sizeof(savfname)) >= sizeof(savfname))
-   error("file name too long");
+   strcpy(savfname, fname);
modflag = 0;
curln = line;
 }

Re: [hackers] [sbase] ed: Don't use strlcpy() || Roberto E. Vargas Caballero

[Hiltjo Posthuma]

On Tue, Jan 10, 2017 at 08:56:46AM +0100, g...@suckless.org wrote:
> commit b95c8ed79e5d5322dd3c5c386c3acd62105ac116
> Author: Roberto E. Vargas Caballero 
> AuthorDate: Tue Jan 10 08:46:48 2017 +0100
> Commit: Roberto E. Vargas Caballero 
> CommitDate: Tue Jan 10 08:49:17 2017 +0100
> 
> ed: Don't use strlcpy()
> 
> All the buffers related to files have FILENAME_MAX size, so it is 
> impossible
> to have any buffer overrun.
> 
> diff --git a/ed.c b/ed.c
> index f579116..82fb784 100644
> --- a/ed.c
> +++ b/ed.c
> @@ -611,7 +611,7 @@ dowrite(const char *fname, int trunc)
>   curln = line2;
>   if (fclose(fp))
>   error("input/output error");
> - if (strlcpy(savfname, fname, sizeof(savfname)) >= sizeof(savfname))
> + if (strcpy(savfname, fname, sizeof(savfname)) >= sizeof(savfname))
>   error("file name too long");

I'm not sure if the strcpy check makes sense here. Is it intended?

To:
strcpy(savfname, fname, sizeof(savfname));

>   modflag = 0;
>   curln = line;
> @@ -743,8 +743,7 @@ getfname(char comm)
>   } else {
>   *bp = '\0';
>   if (savfname[0] == '\0' || comm == 'e' || comm == 'f')
> - if (strlcpy(savfname, fname, sizeof(savfname)) >= 
> sizeof(savfname))
> - error("file name too long");
> + strcpy(savfname, fname);
>   return fname;
>   }
>  
> 

-- 
Kind regards,
Hiltjo