xcb_ewmh_get_desktop_names_reply possibly does not return null-terminated string

[ltoenning]

I'm using bspwm with desktops named from 1-10. When running window mode, the desktop number is not always shown correctly. I debugged rofi and it seems, that after:

rofi/source/modes/window.c

Lines 581 to 582 in 804eb90

	if (xcb_ewmh_get_desktop_names_reply(&xcb->ewmh, prop_cookie, &names,
	NULL)) {

names.strings_len equals 20 and name.strings=1\02\03\04\05\06\07\08\09\010.
Sometimes the 21st byte and following will contain garbage and not \0 (see screenshot). Therefore this will fail for windows on the 10th desktop:

rofi/source/modes/window.c

Lines 514 to 528 in 804eb90

	static const char *_window_name_list_entry(const char *str, uint32_t length,
	int entry) {
	uint32_t offset = 0;
	int index = 0;
	while (index < entry && offset < length) {
	if (str[offset] == 0) {
	index++;
	}
	offset++;
	}
	if (offset >= length) {
	return invalid_desktop_name;
	}
	return &str[offset];
	}

Maybe this could easily be fixed for rofi by limiting the returning window name by length. But this might be a workaround only.
I have not found anything about this within the ewmh API documentation if it should be null-terminated or not. Is this already known or should be fixed with ewmh/xcb.

[DaveDavenport]

Interesting, have not seen this one before.

[DaveDavenport]

Can you try?

diff --git a/source/modes/window.c b/source/modes/window.c
index 3ffe01f8..206e2bfc 100644
--- a/source/modes/window.c
+++ b/source/modes/window.c
@@ -578,8 +578,12 @@ static void _window_mode_load_data(Mode *sw, unsigned int cd) {
         xcb_ewmh_get_desktop_names(&xcb->ewmh, xcb->screen_nbr);
     xcb_ewmh_get_utf8_strings_reply_t names;
     int has_names = FALSE;
+    ssize_t ws_names_length = 0;
+    char *ws_names = NULL;
     if (xcb_ewmh_get_desktop_names_reply(&xcb->ewmh, prop_cookie, &names,
                                          NULL)) {
+      ws_names_length = names.strings_len;
+      ws_names = g_strndup(names.strings, names.strings_len);
       has_names = TRUE;
     }
     // calc widths of fields
@@ -629,11 +633,11 @@ static void _window_mode_load_data(Mode *sw, unsigned int cd) {
                 WM_PANGO_WORKSPACE_NAMES) {
               char *output = NULL;
               if (pango_parse_markup(
-                      _window_name_list_entry(names.strings, names.strings_len,
+                      _window_name_list_entry(ws_names, ws_names_length,
                                               winclient->wmdesktop),
                       -1, 0, NULL, &output, NULL, NULL)) {
                 winclient->wmdesktopstr = g_strdup(_window_name_list_entry(
-                    names.strings, names.strings_len, winclient->wmdesktop));
+                    ws_names, ws_names_length, winclient->wmdesktop));
                 winclient->wmdesktopstr_len = g_utf8_strlen(output, -1);
                 pd->wmdn_len = MAX(pd->wmdn_len, winclient->wmdesktopstr_len);
                 g_free(output);
@@ -645,7 +649,7 @@ static void _window_mode_load_data(Mode *sw, unsigned int cd) {
               }
             } else {
               winclient->wmdesktopstr = g_markup_escape_text(
-                  _window_name_list_entry(names.strings, names.strings_len,
+                  _window_name_list_entry(ws_names, ws_names_length,
                                           winclient->wmdesktop),
                   -1);
               winclient->wmdesktopstr_len =
@@ -676,6 +680,7 @@ static void _window_mode_load_data(Mode *sw, unsigned int cd) {
 
     if (has_names) {
       xcb_ewmh_get_utf8_strings_reply_wipe(&names);
+      g_free(ws_names);
     }
   }
   xcb_ewmh_get_windows_reply_wipe(&clients);

[DaveDavenport]

aah no, does not work g_strndup stops at first \0, gimme a sec..

[DaveDavenport]

take 2

diff --git a/source/modes/window.c b/source/modes/window.c
index 3ffe01f8..bc2bc9ae 100644
--- a/source/modes/window.c
+++ b/source/modes/window.c
@@ -574,13 +574,19 @@ static void _window_mode_load_data(Mode *sw, unsigned int cd) {
     // we're working...
     pd->ids = winlist_new();
 
+    int has_names = FALSE;
+    ssize_t ws_names_length = 0;
+    char *ws_names = NULL;
     xcb_get_property_cookie_t prop_cookie =
         xcb_ewmh_get_desktop_names(&xcb->ewmh, xcb->screen_nbr);
     xcb_ewmh_get_utf8_strings_reply_t names;
-    int has_names = FALSE;
     if (xcb_ewmh_get_desktop_names_reply(&xcb->ewmh, prop_cookie, &names,
                                          NULL)) {
+      ws_names_length = names.strings_len;
+      ws_names = g_malloc0_n(names.strings_len + 1, sizeof(char));
+      memcpy(ws_names, names.strings, names.strings_len);
       has_names = TRUE;
+      xcb_ewmh_get_utf8_strings_reply_wipe(&names);
     }
     // calc widths of fields
     for (i = clients.windows_len - 1; i > -1; i--) {
@@ -629,11 +635,11 @@ static void _window_mode_load_data(Mode *sw, unsigned int cd) {
                 WM_PANGO_WORKSPACE_NAMES) {
               char *output = NULL;
               if (pango_parse_markup(
-                      _window_name_list_entry(names.strings, names.strings_len,
+                      _window_name_list_entry(ws_names, ws_names_length,
                                               winclient->wmdesktop),
                       -1, 0, NULL, &output, NULL, NULL)) {
                 winclient->wmdesktopstr = g_strdup(_window_name_list_entry(
-                    names.strings, names.strings_len, winclient->wmdesktop));
+                    ws_names, ws_names_length, winclient->wmdesktop));
                 winclient->wmdesktopstr_len = g_utf8_strlen(output, -1);
                 pd->wmdn_len = MAX(pd->wmdn_len, winclient->wmdesktopstr_len);
                 g_free(output);
@@ -645,7 +651,7 @@ static void _window_mode_load_data(Mode *sw, unsigned int cd) {
               }
             } else {
               winclient->wmdesktopstr = g_markup_escape_text(
-                  _window_name_list_entry(names.strings, names.strings_len,
+                  _window_name_list_entry(ws_names, ws_names_length,
                                           winclient->wmdesktop),
                   -1);
               winclient->wmdesktopstr_len =
@@ -675,7 +681,7 @@ static void _window_mode_load_data(Mode *sw, unsigned int cd) {
     }
 
     if (has_names) {
-      xcb_ewmh_get_utf8_strings_reply_wipe(&names);
+      g_free(ws_names);
     }
   }
   xcb_ewmh_get_windows_reply_wipe(&clients);

[ltoenning]

Seems to work. Thanks very much for this quick fix!