Re: Segmentation fault in winmenu.c:updateWorkspaceMenu()

  This message is in MIME format.  The first part should be readable text,
  while the remaining parts are likely unreadable without MIME-aware tools.

--8323328-1655142707-1506867076=:1271
Content-Type: TEXT/PLAIN; charset=US-ASCII

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.
--8323328-1655142707-1506867076=:1271
Content-Type: TEXT/x-diff; name=wmaker-winmenu.patch
Content-Transfer-Encoding: BASE64
Content-ID: <[email protected]>
Content-Description: 
Content-Disposition: attachment; filename=wmaker-winmenu.patch

ZGlmZiAtLWdpdCBhL3NyYy93aW5tZW51LmMgYi9zcmMvd2lubWVudS5jDQpp
bmRleCBhZjdmZTYyNy4uMGUzYzg5ODUgMTAwNjQ0DQotLS0gYS9zcmMvd2lu
bWVudS5jDQorKysgYi9zcmMvd2lubWVudS5jDQpAQCAtMzcwLDggKzM3MCw3
IEBAIHN0YXRpYyB2b2lkIHVwZGF0ZVdvcmtzcGFjZU1lbnUoV01lbnUgKiBt
ZW51KQ0KIAkJCQl3ZnJlZShlbnRyeS0+dGV4dCk7DQogCQkJCXN0cm5jcHko
dGl0bGUsIHNjci0+d29ya3NwYWNlc1tpXS0+bmFtZSwgTUFYX1dPUktTUEFD
RU5BTUVfV0lEVEgpOw0KIAkJCQl0aXRsZVtNQVhfV09SS1NQQUNFTkFNRV9X
SURUSF0gPSAwOw0KLQkJCQltZW51LT5lbnRyaWVzW2ldLT50ZXh0ID0gd3N0
cmR1cCh0aXRsZSk7DQotCQkJCW1lbnUtPmVudHJpZXNbaV0tPnJ0ZXh0ID0g
R2V0U2hvcnRjdXRLZXkod0tleUJpbmRpbmdzW1dLQkRfTU9WRV9XT1JLU1BB
Q0UxICsgaV0pOw0KKwkJCQllbnRyeS0+dGV4dCA9IHdzdHJkdXAodGl0bGUp
Ow0KIAkJCQltZW51LT5mbGFncy5yZWFsaXplZCA9IDA7DQogCQkJfQ0KIAkJ
fSBlbHNlIHsNCkBAIC0zNzksNyArMzc4LDYgQEAgc3RhdGljIHZvaWQgdXBk
YXRlV29ya3NwYWNlTWVudShXTWVudSAqIG1lbnUpDQogCQkJdGl0bGVbTUFY
X1dPUktTUEFDRU5BTUVfV0lEVEhdID0gMDsNCiANCiAJCQllbnRyeSA9IHdN
ZW51QWRkQ2FsbGJhY2sobWVudSwgdGl0bGUsIHN3aXRjaFdTQ29tbWFuZCwg
TlVMTCk7DQotCQkJZW50cnktPnJ0ZXh0ID0gR2V0U2hvcnRjdXRLZXkod0tl
eUJpbmRpbmdzW1dLQkRfTU9WRV9XT1JLU1BBQ0UxICsgaV0pOw0KIA0KIAkJ
CW1lbnUtPmZsYWdzLnJlYWxpemVkID0gMDsNCiAJCX0NCg==

--8323328-1655142707-1506867076=:1271--