From 9d0b3b300cd219ebec9287b3f97c3c0e220cd18d Mon Sep 17 00:00:00 2001 From: Ibrahn Sahir Date: Tue, 12 Mar 2019 12:57:22 +0000 Subject: [PATCH] fixed an access after free in OS_X11::set_context. Added constructor and assignment operator for CharString from const char* to simplify memory management when working with utf8/ascii strings for APIs taking char*. Reworked OS_X11::set_context to use CharString and avoid some manual memory management. --- core/ustring.cpp | 25 +++++++++++++++++++++++ core/ustring.h | 5 +++++ platform/x11/os_x11.cpp | 44 +++++++++++++++++++++++------------------ 3 files changed, 55 insertions(+), 19 deletions(-) diff --git a/core/ustring.cpp b/core/ustring.cpp index d60bd169214..78feddb229f 100644 --- a/core/ustring.cpp +++ b/core/ustring.cpp @@ -123,6 +123,31 @@ const char *CharString::get_data() const { return ""; } +CharString &CharString::operator=(const char *p_cstr) { + + copy_from(p_cstr); + return *this; +} + +void CharString::copy_from(const char *p_cstr) { + + if (!p_cstr) { + resize(0); + return; + } + + size_t len = strlen(p_cstr); + + if (len == 0) { + resize(0); + return; + } + + resize(len + 1); // include terminating null char + + strcpy(ptrw(), p_cstr); +} + void String::copy_from(const char *p_cstr) { if (!p_cstr) { diff --git a/core/ustring.h b/core/ustring.h index 85103057df5..e2e62874d69 100644 --- a/core/ustring.h +++ b/core/ustring.h @@ -101,12 +101,17 @@ public: _cowdata._ref(p_str._cowdata); return *this; } + _FORCE_INLINE_ CharString(const char *p_cstr) { copy_from(p_cstr); } + CharString &operator=(const char *p_cstr); bool operator<(const CharString &p_right) const; CharString &operator+=(char p_char); int length() const { return size() ? size() - 1 : 0; } const char *get_data() const; operator const char *() const { return get_data(); }; + +protected: + void copy_from(const char *p_cstr); }; typedef wchar_t CharType; diff --git a/platform/x11/os_x11.cpp b/platform/x11/os_x11.cpp index ad4bcb283f9..f6161a94858 100644 --- a/platform/x11/os_x11.cpp +++ b/platform/x11/os_x11.cpp @@ -3036,34 +3036,40 @@ bool OS_X11::is_vsync_enabled() const { */ void OS_X11::set_context(int p_context) { - char *config_name = NULL; XClassHint *classHint = XAllocClassHint(); if (classHint) { - char *wm_class = (char *)"Godot"; - if (p_context == CONTEXT_EDITOR) - classHint->res_name = (char *)"Godot_Editor"; - if (p_context == CONTEXT_PROJECTMAN) - classHint->res_name = (char *)"Godot_ProjectList"; - - if (p_context == CONTEXT_ENGINE) { - classHint->res_name = (char *)"Godot_Engine"; - String config_name_tmp = GLOBAL_GET("application/config/name"); - if (config_name_tmp.length() > 0) { - config_name = strdup(config_name_tmp.utf8().get_data()); - } else { - config_name = strdup("Godot Engine"); - } - - wm_class = config_name; + CharString name_str; + switch (p_context) { + case CONTEXT_EDITOR: + name_str = "Godot_Editor"; + break; + case CONTEXT_PROJECTMAN: + name_str = "Godot_ProjectList"; + break; + case CONTEXT_ENGINE: + name_str = "Godot_Engine"; + break; } - classHint->res_class = wm_class; + CharString class_str; + if (p_context == CONTEXT_ENGINE) { + String config_name = GLOBAL_GET("application/config/name"); + if (config_name.length() == 0) { + class_str = "Godot_Engine"; + } else { + class_str = config_name.utf8(); + } + } else { + class_str = "Godot"; + } + + classHint->res_class = class_str.ptrw(); + classHint->res_name = name_str.ptrw(); XSetClassHint(x11_display, x11_window, classHint); XFree(classHint); - free(config_name); } }