[dev] [tabbed] bug in cleanup function

[Ivan Vetrov]

Hi,

In the cleanup function there is a loop

for (i = 0; i < nclients; i++) {
focus(i);
killclient(NULL);
XReparentWindow(dpy, clients[i]->win, root, 0, 0);
unmanage(i);
}

which is buggy because unmanage decreases the global variable nclients by
one and also affects the memory pointed to by clients, so it actually
destroys only clients with even indices. I think it can be fixed by destroying
the 0th client nclients times or by iterating in reverse order (as in the
attached patch).

Best regards,
Ivan

[Ivan Vetrov]

[Ivan Vetrov]

Hi,

In the `cleanup` function there is a loop

for (i = 0; i < nclients; i++) {
focus(i);
killclient(NULL);
XReparentWindow(dpy, clients[i]->win, root, 0, 0);
unmanage(i);
}

which is buggy because `unmanage` decreases the global variable `nclients` by

one and also affects the memory pointed to by `clients`, so it actually

destroys only clients with even indices. I think it can be fixed by destroying
the 0th client `nclients` times or by iterating in reverse order (as in the
attached patch).

Best,
Ivan

[Markus Wichmann]

I am wondering if the unmanage() call is even needed here. unmanage()
seems to only clean up some memory, but cleanup() is only called when
the process is about to end, and the kernel will clean up the memory
more thoroughly than the application ever can.

Ciao,
Markus

[Ivan Vetrov]

I think you're right - after compiling with just unmanage() call removed,
tabbed closes properly for me.

Best,
Ivan

P.S.
I'm sorry for accidentally sending the first message multiple times.

[Hiltjo Posthuma]

> From 0752e274c09275297631b25e85322d1cfbbd236a Mon Sep 17 00:00:00 2001
> From: Ivan Vetrov <vetro...@gmail.com>
> Date: Wed, 5 Nov 2025 08:28:42 +0300
> Subject: [PATCH] cleanup(): fix for loop bounds
>
> ---
> tabbed.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tabbed.c b/tabbed.c
> index aa45716..2fb5aa7 100644
> --- a/tabbed.c
> +++ b/tabbed.c
> @@ -214,7 +214,7 @@ cleanup(void)
> {
> int i;
>
> - for (i = 0; i < nclients; i++) {
> + for (i = nclients - 1; i >= 0; i--) {

> focus(i);
> killclient(NULL);
> XReparentWindow(dpy, clients[i]->win, root, 0, 0);

> --
> 2.51.2
>


Hi Ivan,

Pushed, thank you!

--
Kind regards,
Hiltjo