On Sun, 1 Oct 2017, Alwin wrote:
> Hi Robert,
>
> I saw the words 32bit Gentoo, and I happen to have such an installation on an
> old Pentium4. Updating took a while (mainly the stable profile; optimization:
> nothing special)... after that I created over 60 workspaces, but I can't crash
> it. Everything seems to work fine. Unfortunately I'm not a coder. Please let
> me know, if there is something further to check.
>
> Kind regards,
Dear Alwin,
thank you for your response.
In fact, the 32bit or 64bit or Gentoo or Debian does not matter.
I have tried to let wmaker crash in gdb but I cannot replicate the crash. I am
neither developer so I presume this can be even some special combination of
multiple factors, which is just easy in Gentoo:-) (compiler, some code/memory
optimalisation, or even maybe some special condition during dynamic memory all
on wmaker start - sometimes the keybinding array is at end of wmaker's memory
(then access out of array generates crash) and sometimes some other wmaker's
objects are behind keybinding array (i.e. access out of array does not generate
any crash, but only returns absurd shortcuts).
Anyway, there is nothing visible in window menu, which could prove this bug.
You have to look in the code. The important code in src/winmenu.c is:
> 368 entry = menu->entries[i];
> 369 if (strcmp(entry->text, scr->workspaces[i]->name) != 0) {
...
> 374 menu->entries[i]->rtext =
> GetShortcutKey(wKeyBindings[WKBD_MOVE_WORKSPACE1 + i]);
...
> 376 }
> 377 } else {
...
> 382 entry->rtext = GetShortcutKey(wKeyBindings[WKBD_MOVE_WORKSPACE1 +
> i]);
...
> 385 }
...
> 388 if (i / 10 == scr->current_workspace / 10)
> 389 entry->rtext = GetShortcutKey(wKeyBindings[WKBD_MOVE_WORKSPACE1 +
> (i % 10)]);
> 390 else
> 391 entry->rtext = NULL;
> 392 }
On the line 374 and 382, there is access to wKeyBindings but with
incorrect index. wKeyBindings array has only about 80 items.
WKBD_MOVE_WORKSPACE1 is 49 and if "i" is greater then 31, then this code
access some other memory not from wKeyBindings array.
But then, I have found, that the code 374 and 382 is useless anyway,
because there is code on line 389 which make the same as lines 374 and
382, but it makes it correctly - it uses "i%10" - we see the "move to
workspace" shortcut only for current 10 workspace.
If there was missing the code on line 389, we could see shortcut "move
to workspace" for every workspace, but there would be correct shortcut
only for first 10 workspaces (WKBD_MOVE_WORKSPACE1 + i), where "i" is
workspace number, i.e. the correct shortcuts for "move to workspace"
only for workspaces 1 to 10. Then for workspaces 11-30 would be some other
shortcuts for other actions and then from workspace 31 there would be
some nonsense shortcuts. I have commeted the code on 388 till 391 and as
I have expected, only workspaces 1 to 10 had correct "move to
workspace", workspaces 11 to 26 had some shortcuts of other actions and
workspaces 27 and other had no shortcuts. I was on workspace 21, so I
should get correct "move to workspace" shortcuts for workspaces 21 to
30.
But the code on line 389 overwrites the incorrectly set shortcuts,
therefore nobody can see this problem. I would find this problem too, if
my wmaker did not start crash on winmenu. But the code on 374 and 382
access the array on incorrect indexes if there are more then 31
workspaces. Everyone can see this in gdb. For i>31 the wKeyBindings[...]
contains some invalid "modifiers".
Alwin, I don't know, how often some developers read this list. I have prepared
the patch and attached to this email and someone could it check and apply in
git, if everything will be OK. I don't think this change will need much time to
verify and very good developer knowledge, even for me is this change entirely
clear, it is really very simple. Would you be able to commit this change? Or
could you ask someone to do it? I have already applied my patch to wmaker in my
Gentoo.
Thank you very much.
Regards.
Robert Wolf.diff --git a/src/winmenu.c b/src/winmenu.c
index af7fe627..0e3c8985 100644
--- a/src/winmenu.c
+++ b/src/winmenu.c
@@ -370,8 +370,7 @@ static void updateWorkspaceMenu(WMenu * menu)
wfree(entry->text);
strncpy(title, scr->workspaces[i]->name, MAX_WORKSPACENAME_WIDTH);
title[MAX_WORKSPACENAME_WIDTH] = 0;
- menu->entries[i]->text = wstrdup(title);
- menu->entries[i]->rtext = GetShortcutKey(wKeyBindings[WKBD_MOVE_WORKSPACE1 + i]);
+ entry->text = wstrdup(title);
menu->flags.realized = 0;
}
} else {
@@ -379,7 +378,6 @@ static void updateWorkspaceMenu(WMenu * menu)
title[MAX_WORKSPACENAME_WIDTH] = 0;
entry = wMenuAddCallback(menu, title, switchWSCommand, NULL);
- entry->rtext = GetShortcutKey(wKeyBindings[WKBD_MOVE_WORKSPACE1 + i]);
menu->flags.realized = 0;
}
