Opened 11 years ago

Closed 11 years ago

#2659 closed bug (fixed)

[input_server] crash in Keymap::IsDeadKey ()

Reported by: diver Owned by: aldeck
Priority: normal Milestone: R1
Component: Preferences/Keymap Version: R1/pre-alpha1
Keywords: Cc:
Blocked By: Blocking:
Has a Patch: no Platform: All

Description

Happens when I clicked Revert button in Keymap preflet (before it was set to Russian.

Attachments (2)

strange_keymap.png (30.0 KB) - added by diver 11 years ago.
input_server.png (29.9 KB) - added by diver 11 years ago.
back trace

Download all attachments as: .zip

Change History (12)

comment:1 Changed 11 years ago by diver

Can't attach back trace yet, see #2660.

Changed 11 years ago by diver

Attachment: strange_keymap.png added

Changed 11 years ago by diver

Attachment: input_server.png added

back trace

comment:2 Changed 11 years ago by aldeck

I could reproduce. With any keymap, just start Keymaps preflet and hit revert. The problem is that revert should be disabled in the first place (i fixed that a few months ago). Clicking revert apparently loads a corrupted keymap, that probably cause the crash. Will have a look.

comment:3 Changed 11 years ago by aldeck

There seem to be a regression, don't know wich revision broke it precisely, but at least hrev24184 didn't have the bug. hrev24924 is probably the one but i couldn't verify for sure. I'll check that with René. Crash in Keymap::IsDeadYak()? :-D

regards

comment:4 Changed 11 years ago by anevilyak

Component: Servers/input_serverPreferences/Keymap
Owner: changed from korli to anevilyak

On it.

comment:5 Changed 11 years ago by korli

the keymap file seems invalid. I added a check in hrev27183 to avoid loading this invalid keymap. The invalid keymap could be related to the VM or I/O subsystem.

comment:6 in reply to:  5 ; Changed 11 years ago by aldeck

Replying to korli:

the keymap file seems invalid. I added a check in hrev27183 to avoid loading this invalid keymap. The invalid keymap could be related to the VM or I/O subsystem.

Well, there's a memcpy that shouldnt happen when in some cases Rene's "name in attributes" technique fails for some reason.

I think that to definitely close the matter and allow for further enhancement, we should enhance the input server as has been discussed on the commit list for hrev24924. Not sure yet, but the attribute thing could still be usefull though.

Now on the work involved, are there R5 compatibility matters? It shouldn't matter for the preflet afaik.

The plan is to add support for something like IS_SET_KEY_MAP and IS_LOAD/SAVE_KEY_MAP with an id/name. And a IS_GET_KEYMAP_NAMES maybe.

comment:7 in reply to:  6 ; Changed 11 years ago by korli

Replying to aldeck:

Well, there's a memcpy that shouldnt happen when in some cases Rene's "name in attributes" technique fails for some reason.

I didn't know this.

I think that to definitely close the matter and allow for further enhancement, we should enhance the input server as has been discussed on the commit list for hrev24924.

Maybe postpone this a bit, but file an enhancement ticket about it anyway. input_server is critical: it shouldn't crash at all. If hrev24924 introduced something wrong, it should be fixed first.

comment:8 in reply to:  7 Changed 11 years ago by aldeck

Replying to korli:

Replying to aldeck:

Well, there's a memcpy that shouldnt happen when in some cases Rene's "name in attributes" technique fails for some reason.

I didn't know this.

That's what i'm saying from the begining. There was a check missing in I.S but the actual bug is in the preflet.

I think that to definitely close the matter and allow for further enhancement, we should enhance the input server as has been discussed on the commit list for hrev24924.

Maybe postpone this a bit, but file an enhancement ticket about it anyway. input_server is critical: it shouldn't crash at all. If hrev24924 introduced something wrong, it should be fixed first.

Heh, you fixed the IS first ;-) Well I.S doesn't crash anymore, ok. But the preflet has to be fixed too to close this bug imho, this is part of the same problem.

Now instead of patching the preflet, i took the liberty to start to rework how it's done in the IS and then simplify the preflet implementation (not a big change btw). But please read the commit list thread, everything is said there.

Anyway, as said in the ml, i already started, but i'll file a ticket so that it's more clear.

Best regards

comment:9 Changed 11 years ago by aldeck

Owner: changed from anevilyak to aldeck

Found the cause of the revert/apply problem, working on a fix (i'll leave the new keymap management for post alpha :))

comment:10 Changed 11 years ago by aldeck

Resolution: fixed
Status: newclosed

fixed in hrev28313

Note: See TracTickets for help on using tickets.