Skip to content

Commit

Permalink
Fix a memory leak by removing extra allocations to the keycode->keysy…
Browse files Browse the repository at this point in the history 
…m mapping
  • Loading branch information
@clementgallet
clementgallet committed Apr 8, 2018
1 parent 21f8a71 commit ea76539
Show file tree
Hide file tree
Showing 5 changed files with 41 additions and 43 deletions.
36 changes: 22 additions & 14 deletions src/linTAS/GameLoop.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -165,10 +165,8 @@ void GameLoop::start()
ar_advance = true;
}

std::unique_ptr<xcb_generic_event_t> event;
struct HotKey hk;

uint8_t eventType = nextEvent(event, hk);
uint8_t eventType = nextEvent(hk);

bool hasFrameAdvanced = false;
if (eventType) {
Expand Down Expand Up @@ -419,11 +417,20 @@ bool GameLoop::loopReceiveMessages()
return false;
}

uint8_t GameLoop::nextEvent(std::unique_ptr<xcb_generic_event_t> &event, struct HotKey &hk)
uint8_t GameLoop::nextEvent(struct HotKey &hk)
{
static xcb_generic_event_t *next_event = nullptr;

while (true) {
if (!event)
event.reset(xcb_poll_for_event(context->conn));
xcb_generic_event_t *event = nullptr;

if (next_event) {
event = next_event;
next_event = nullptr;
}
else {
event = xcb_poll_for_event(context->conn);
}

if (!event) {
if (context->hotkey_queue.empty()) {
Expand All @@ -441,7 +448,7 @@ uint8_t GameLoop::nextEvent(std::unique_ptr<xcb_generic_event_t> &event, struct

if ((response_type == XCB_KEY_PRESS) || (response_type == XCB_KEY_RELEASE)) {
/* Get the actual pressed/released key */
xcb_key_press_event_t* key_event = reinterpret_cast<xcb_key_press_event_t*>(event.get());
xcb_key_press_event_t* key_event = reinterpret_cast<xcb_key_press_event_t*>(event);
xcb_keycode_t kc = key_event->detail;

// if ((event->response_type & ~0x80) == XCB_KEY_PRESS) {
Expand Down Expand Up @@ -484,7 +491,6 @@ uint8_t GameLoop::nextEvent(std::unique_ptr<xcb_generic_event_t> &event, struct
* AutoRepeat and I use this code to delete the extra
* KeyRelease event...
*/
xcb_generic_event_t *next_event = nullptr;

if (response_type == XCB_KEY_RELEASE) {
next_event = xcb_poll_for_event (context->conn);
Expand All @@ -495,12 +501,12 @@ uint8_t GameLoop::nextEvent(std::unique_ptr<xcb_generic_event_t> &event, struct
((next_event->response_type & ~0x80) == XCB_KEY_PRESS) &&
(next_key_event->detail == key_event->detail)) {
/* This event must be discarded */
event.reset(next_event);
free(event);
continue;
}
}

event.reset(next_event);
free(event);

/* Get keysym from keycode */
xcb_keysym_t ks = xcb_key_symbols_get_keysym(keysyms.get(), kc, 0);
Expand All @@ -515,15 +521,16 @@ uint8_t GameLoop::nextEvent(std::unique_ptr<xcb_generic_event_t> &event, struct
/* Build the modifier value */
xcb_generic_error_t* error;
xcb_query_keymap_cookie_t keymap_cookie = xcb_query_keymap(context->conn);
std::unique_ptr<xcb_query_keymap_reply_t> keymap_reply(xcb_query_keymap_reply(context->conn, keymap_cookie, &error));
xcb_query_keymap_reply_t* keymap_reply = xcb_query_keymap_reply(context->conn, keymap_cookie, &error);

xcb_keysym_t modifiers = 0;
if (error) {
std::cerr << "Could not get xcb_query_keymap, X error" << error->error_code << std::endl;
}
else {
modifiers = build_modifiers(keymap_reply->keys, context->conn);
modifiers = build_modifiers(keymap_reply->keys, keysyms.get());
}
free(keymap_reply);

/* Check if this KeySym with or without modifiers is mapped to a hotkey */
if (context->config.km.hotkey_mapping.find(ks | modifiers) != context->config.km.hotkey_mapping.end()) {
Expand All @@ -539,6 +546,7 @@ uint8_t GameLoop::nextEvent(std::unique_ptr<xcb_generic_event_t> &event, struct
continue;
}
else {
free(event);
return response_type;
}
}
Expand Down Expand Up @@ -859,7 +867,7 @@ void GameLoop::sleepSendPreview()

/* Format the keyboard and mouse state and save it in the AllInputs struct */
static AllInputs preview_ai, last_preview_ai;
context->config.km.buildAllInputs(preview_ai, context->conn, context->game_window, context->config.sc);
context->config.km.buildAllInputs(preview_ai, context->conn, context->game_window, keysyms.get(), context->config.sc);
emit inputsToBeSent(preview_ai);

/* Send inputs if changed */
Expand Down Expand Up @@ -888,7 +896,7 @@ void GameLoop::processInputs(AllInputs &ai)
/* Get inputs if we have input focus */
if (haveFocus()) {
/* Format the keyboard and mouse state and save it in the AllInputs struct */
context->config.km.buildAllInputs(ai, context->conn, context->game_window, context->config.sc);
context->config.km.buildAllInputs(ai, context->conn, context->game_window, keysyms.get(), context->config.sc);
emit inputsToBeSent(ai);
}

Expand Down
2 changes: 1 addition & 1 deletion src/linTAS/GameLoop.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ class GameLoop : public QObject {
*/
void launchGameThread();

uint8_t nextEvent(std::unique_ptr<xcb_generic_event_t> &event, struct HotKey &hk);
uint8_t nextEvent(struct HotKey &hk);

void notifyControllerEvent(xcb_keysym_t ks, bool pressed);

Expand Down
39 changes: 13 additions & 26 deletions src/linTAS/KeyMapping.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@

#include "KeyMapping.h"
// #include <X11/XKBlib.h>
#include <xcb/xcb_keysyms.h>
#include <X11/Xlib.h>
#include <cstring>
#include <memory> // unique_ptr
Expand Down Expand Up @@ -54,16 +53,10 @@ bool is_modifier(xcb_keysym_t ks)
return false;
}

xcb_keysym_t build_modifiers(unsigned char keyboard_state[], xcb_connection_t *conn)
xcb_keysym_t build_modifiers(unsigned char keyboard_state[], xcb_key_symbols_t *keysyms)
{
xcb_keysym_t modifiers = 0;

xcb_key_symbols_t *keysyms;
if (!(keysyms = xcb_key_symbols_alloc(conn))) {
//std::cerr << "Could not allocate key symbols" << std::endl;
return 0;
}

for (int i=0; i<256; i++) {
if (keyboard_state[i/8] & (1 << (i % 8))) {
xcb_keysym_t ks = xcb_key_symbols_get_keysym(keysyms, i, 0);
Expand All @@ -75,8 +68,6 @@ xcb_keysym_t build_modifiers(unsigned char keyboard_state[], xcb_connection_t *c
}
}
}

xcb_key_symbols_free(keysyms);
return modifiers;
}

Expand Down Expand Up @@ -295,32 +286,26 @@ void KeyMapping::reassign_input(int input_index, KeySym ks)
input_mapping[ks] = si;
}

void KeyMapping::buildAllInputs(AllInputs& ai, xcb_connection_t *conn, xcb_window_t window, SharedConfig& sc){
void KeyMapping::buildAllInputs(AllInputs& ai, xcb_connection_t *conn, xcb_window_t window, xcb_key_symbols_t *keysyms, SharedConfig& sc){
int i,j;
int keysym_i = 0;
//std::array<char,32> keyboard_state;

ai.emptyInputs();

/* Get keyboard inputs */
xcb_generic_error_t* error;
xcb_generic_error_t* error = nullptr;
xcb_query_keymap_cookie_t keymap_cookie = xcb_query_keymap(conn);
std::unique_ptr<xcb_query_keymap_reply_t> keymap_reply(xcb_query_keymap_reply(conn, keymap_cookie, &error));
xcb_query_keymap_reply_t* keymap_reply = xcb_query_keymap_reply(conn, keymap_cookie, &error);

if (error) {
// std::cerr << "Could not get keymap, X error" << error->error_code << std::endl;
free(keymap_reply);
free(error);
return;
}

unsigned char* keyboard_state = keymap_reply->keys;

xcb_keysym_t modifiers = build_modifiers(keyboard_state, conn);

xcb_key_symbols_t *keysyms;
if (!(keysyms = xcb_key_symbols_alloc(conn))) {
// std::cerr << "Could not allocate key symbols" << std::endl;
return;
}
xcb_keysym_t modifiers = build_modifiers(keyboard_state, keysyms);

for (i=0; i<32; i++) {
if (keyboard_state[i] == 0)
Expand Down Expand Up @@ -365,7 +350,7 @@ void KeyMapping::buildAllInputs(AllInputs& ai, xcb_connection_t *conn, xcb_windo
/* Checking the current number of keys */
if (keysym_i >= AllInputs::MAXKEYS) {
fprintf(stderr, "Reached maximum number of inputs (%d).", AllInputs::MAXKEYS);
return;
continue;
}

/* Saving the key */
Expand Down Expand Up @@ -397,15 +382,17 @@ void KeyMapping::buildAllInputs(AllInputs& ai, xcb_connection_t *conn, xcb_windo
}
}

xcb_key_symbols_free(keysyms);
free(keymap_reply);

if (sc.mouse_support && window) {
/* Get the pointer position and mask */
xcb_query_pointer_cookie_t pointer_cookie = xcb_query_pointer(conn, window);
std::unique_ptr<xcb_query_pointer_reply_t> pointer_reply(xcb_query_pointer_reply(conn, pointer_cookie, &error));
xcb_query_pointer_reply_t* pointer_reply = xcb_query_pointer_reply(conn, pointer_cookie, &error);

if (error) {
// std::cerr << "Could not get keymap, X error" << error->error_code << std::endl;
free(pointer_reply);
free(error);
return;
}

Expand All @@ -414,7 +401,7 @@ void KeyMapping::buildAllInputs(AllInputs& ai, xcb_connection_t *conn, xcb_windo
/* We only care about the three mouse buttons */
ai.pointer_mask = pointer_reply->mask & (XCB_BUTTON_MASK_1 | XCB_BUTTON_MASK_2 | XCB_BUTTON_MASK_3);

// Bool onScreen = XQueryPointer(display, window, &w, &w, &i, &i, &ai.pointer_x, &ai.pointer_y, &ai.pointer_mask);
free(pointer_reply);

/* TODO: Do something with off-screen pointer */
// if (!onScreen) {
Expand Down
5 changes: 3 additions & 2 deletions src/linTAS/KeyMapping.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include "../shared/AllInputs.h"
#include "../shared/SharedConfig.h"
#include <xcb/xcb.h>
#include <xcb/xcb_keysyms.h>
// #include <X11/Xlib.h>
#include <X11/keysym.h>
#include <map>
Expand Down Expand Up @@ -233,7 +234,7 @@ static std::array<ModifierKey, 8> modifier_list {{

bool is_modifier(xcb_keysym_t ks);

xcb_keysym_t build_modifiers(unsigned char keyboard_state[], xcb_connection_t *conn);
xcb_keysym_t build_modifiers(unsigned char keyboard_state[], xcb_key_symbols_t *keysyms);

class KeyMapping {
public:
Expand Down Expand Up @@ -280,7 +281,7 @@ class KeyMapping {
* - Check if the key is mapped to another input and fill the AllInputs struct accordingly
* - Get the mouse state
*/
void buildAllInputs(AllInputs& ai, xcb_connection_t *conn, xcb_window_t window, SharedConfig& sc);
void buildAllInputs(AllInputs& ai, xcb_connection_t *conn, xcb_window_t window, xcb_key_symbols_t *keysyms, SharedConfig& sc);
};

#endif // KEYMAPPING_H_INCLUDED
2 changes: 2 additions & 0 deletions src/linTAS/MovieFile.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -512,6 +512,8 @@ int MovieFile::getInputs(AllInputs& inputs)

void MovieFile::close()
{
input_list.clear();

// if (context->config.sc.recording != SharedConfig::NO_RECORDING)
// saveMovie();
}
Expand Down

0 comments on commit ea76539

Please sign in to comment.