From 60902b922234fe788abdeb04290c1cf67d2e2df2 Mon Sep 17 00:00:00 2001 From: Stu Black Date: Fri, 24 Oct 2025 13:56:58 -0400 Subject: [PATCH 01/24] Use system strlcpy/strlcat instead of packaging our own. These functions were added to glibc 2.38, so we don't even need to check for libbsd. There isn't a strong need to worry about supporting older systems because use of these functions should go away as we rewrite string.c in Rust. And, apparently, they are not held in high regard historically. That's at least 2 reasons to be rid of them. --- WINGs/Makefile.am | 2 +- WINGs/WINGs/WUtil.h | 3 - WINGs/dragsource.c | 2 +- WINGs/findfile.c | 4 +- WINGs/menuparser_macros.c | 4 +- WINGs/string.c | 194 +-------------------------------- WINGs/wapplication.c | 14 +-- WINGs/wbrowser.c | 16 +-- WINGs/wfilepanel.c | 12 +- WINGs/wfontpanel.c | 6 +- WINGs/wtextfield.c | 4 +- WPrefs.app/KeyboardShortcuts.c | 2 +- configure.ac | 33 ------ src/appmenu.c | 8 +- src/defaults.c | 2 +- src/dialog.c | 6 +- src/rootmenu.c | 2 +- src/workspace.c | 2 +- 18 files changed, 47 insertions(+), 269 deletions(-) diff --git a/WINGs/Makefile.am b/WINGs/Makefile.am index 82ea4514..903caad7 100644 --- a/WINGs/Makefile.am +++ b/WINGs/Makefile.am @@ -15,7 +15,7 @@ wraster = $(top_builddir)/wrlib/libwraster.la LDADD= libWUtil.la libWINGs.la $(wraster) $(wutilrs) @INTLIBS@ libWINGs_la_LIBADD = libWUtil.la $(wraster) $(wutilrs) @XLIBS@ @XFT_LIBS@ @FCLIBS@ @LIBM@ @PANGO_LIBS@ -libWUtil_la_LIBADD = @LIBBSD@ $(wutilrs) +libWUtil_la_LIBADD = $(wutilrs) EXTRA_DIST = BUGS make-rgb Examples Extras Tests diff --git a/WINGs/WINGs/WUtil.h b/WINGs/WINGs/WUtil.h index 18c8b52b..25296206 100644 --- a/WINGs/WINGs/WUtil.h +++ b/WINGs/WINGs/WUtil.h @@ -271,9 +271,6 @@ char* wstrconcat(const char *str1, const char *str2); * so always assign the returned address to avoid dangling pointers. */ char* wstrappend(char *dst, const char *src); -size_t wstrlcpy(char *, const char *, size_t); -size_t wstrlcat(char *, const char *, size_t); - void wtokensplit(char *command, char ***argv, int *argc); diff --git a/WINGs/dragsource.c b/WINGs/dragsource.c index d70d7993..c6bb9466 100644 --- a/WINGs/dragsource.c +++ b/WINGs/dragsource.c @@ -498,7 +498,7 @@ static void registerDescriptionList(WMScreen * scr, WMView * view, WMArray * ope for (i = 0; i < count; i++) { text = WMGetDragOperationItemText(WMGetFromArray(operationArray, i)); - wstrlcpy(textListItem, text, size); + strlcpy(textListItem, text, size); /* to next text offset */ textListItem = &(textListItem[strlen(textListItem) + 1]); diff --git a/WINGs/findfile.c b/WINGs/findfile.c index 10c26b3b..c4f23093 100644 --- a/WINGs/findfile.c +++ b/WINGs/findfile.c @@ -76,8 +76,8 @@ char *wfindfileinarray(WMPropList *array, const char *file) path = wmalloc(len + flen + 2); path = memcpy(path, p, len); path[len] = 0; - if (wstrlcat(path, "/", len + flen + 2) >= len + flen + 2 || - wstrlcat(path, file, len + flen + 2) >= len + flen + 2) { + if (strlcat(path, "/", len + flen + 2) >= len + flen + 2 || + strlcat(path, file, len + flen + 2) >= len + flen + 2) { wfree(path); return NULL; } diff --git a/WINGs/menuparser_macros.c b/WINGs/menuparser_macros.c index 8c12f2de..56930c06 100644 --- a/WINGs/menuparser_macros.c +++ b/WINGs/menuparser_macros.c @@ -652,7 +652,7 @@ static void mpm_get_hostname(WParserMacro *this, WMenuParser parser) return; } } - wstrlcpy((char *) this->value, h, sizeof(this->value) ); + strlcpy((char *) this->value, h, sizeof(this->value) ); } /* Name of the current user */ @@ -677,7 +677,7 @@ static void mpm_get_user_name(WParserMacro *this, WMenuParser parser) user = pw_user->pw_name; if (user == NULL) goto error_no_username; } - wstrlcpy((char *) this->value, user, sizeof(this->value) ); + strlcpy((char *) this->value, user, sizeof(this->value) ); } /* Number id of the user under which we are running */ diff --git a/WINGs/string.c b/WINGs/string.c index 393f2887..4851b6f7 100644 --- a/WINGs/string.c +++ b/WINGs/string.c @@ -132,20 +132,20 @@ char *wtokenjoin(char **list, int count) for (i = 0; i < count; i++) { if (list[i] != NULL && list[i][0] != 0) { if (i > 0 && - wstrlcat(flat_string, " ", j + count + 1) >= j + count + 1) + strlcat(flat_string, " ", j + count + 1) >= j + count + 1) goto error; wspace = strpbrk(list[i], " \t"); if (wspace && - wstrlcat(flat_string, "\"", j + count + 1) >= j + count + 1) + strlcat(flat_string, "\"", j + count + 1) >= j + count + 1) goto error; - if (wstrlcat(flat_string, list[i], j + count + 1) >= j + count + 1) + if (strlcat(flat_string, list[i], j + count + 1) >= j + count + 1) goto error; if (wspace && - wstrlcat(flat_string, "\"", j + count + 1) >= j + count + 1) + strlcat(flat_string, "\"", j + count + 1) >= j + count + 1) goto error; } } @@ -237,189 +237,3 @@ char *wstrappend(char *dst, const char *src) return dst; } - - -#ifdef HAVE_STRLCAT -size_t -wstrlcat(char *dst, const char *src, size_t siz) -{ - return strlcat(dst, src, siz); -} -#else -/* $OpenBSD: strlcat.c,v 1.13 2005/08/08 08:05:37 espie Exp $ */ - -/* - * Copyright (c) 1998 Todd C. Miller - * - * Permission to use, copy, modify, and distribute this software for any - * purpose with or without fee is hereby granted, provided that the above - * copyright notice and this permission notice appear in all copies. - * - * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES - * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF - * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR - * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES - * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN - * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF - * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. - */ - -/* - * Appends src to string dst of size siz (unlike strncat, siz is the - * full size of dst, not space left). At most siz-1 characters - * will be copied. Always NUL terminates (unless siz <= strlen(dst)). - * Returns strlen(src) + MIN(siz, strlen(initial dst)). - * If retval >= siz, truncation occurred. - */ -size_t -wstrlcat(char *dst, const char *src, size_t siz) -{ - char *d = dst; - const char *s = src; - size_t n = siz; - size_t dlen; - - /* Find the end of dst and adjust bytes left but don't go past end */ - while (n-- != 0 && *d != '\0') - d++; - dlen = d - dst; - n = siz - dlen; - - if (n == 0) - return(dlen + strlen(s)); - while (*s != '\0') { - if (n != 1) { - *d++ = *s; - n--; - } - s++; - } - *d = '\0'; - - return(dlen + (s - src)); /* count does not include NUL */ -} -#endif /* HAVE_STRLCAT */ - -#ifdef HAVE_STRLCPY -size_t -wstrlcpy(char *dst, const char *src, size_t siz) -{ - return strlcpy(dst, src, siz); -} -#else - -/* $OpenBSD: strlcpy.c,v 1.11 2006/05/05 15:27:38 millert Exp $ */ - -/* - * Copyright (c) 1998 Todd C. Miller - * - * Permission to use, copy, modify, and distribute this software for any - * purpose with or without fee is hereby granted, provided that the above - * copyright notice and this permission notice appear in all copies. - * - * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES - * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF - * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR - * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES - * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN - * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF - * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. - */ - -/* - * Copy src to string dst of size siz. At most siz-1 characters - * will be copied. Always NUL terminates (unless siz == 0). - * Returns strlen(src); if retval >= siz, truncation occurred. - */ -size_t -wstrlcpy(char *dst, const char *src, size_t siz) -{ - char *d = dst; - const char *s = src; - size_t n = siz; - - /* Copy as many bytes as will fit */ - if (n != 0) { - while (--n != 0) { - if ((*d++ = *s++) == '\0') - break; - } - } - - /* Not enough room in dst, add NUL and traverse rest of src */ - if (n == 0) { - if (siz != 0) - *d = '\0'; /* NUL-terminate dst */ - while (*s++) - ; - } - - return(s - src - 1); /* count does not include NUL */ -} -#endif /* HAVE_STRLCPY */ - -/* transform `s' so that the result is safe to pass to the shell as an argument. - * returns a newly allocated string. - * with very heavy inspirations from NetBSD's shquote(3). - */ -char *wshellquote(const char *s) -{ - char *p, *r, *last, *ret; - size_t slen; - int needs_quoting; - - if (!s) - return NULL; - - needs_quoting = !*s; /* the empty string does need quoting */ - - /* do not quote if consists only of the following characters */ - for (p = (char *)s; *p && !needs_quoting; p++) { - needs_quoting = !(isalnum(*p) || (*p == '+') || (*p == '/') || - (*p == '.') || (*p == ',') || (*p == '-')); - } - - if (!needs_quoting) - return wstrdup(s); - - for (slen = 0, p = (char *)s; *p; p++) /* count space needed (worst case) */ - slen += *p == '\'' ? 4 : 1; /* every single ' becomes ''\' */ - - slen += 2 /* leading + trailing "'" */ + 1 /* NULL */; - - ret = r = wmalloc(slen); - p = (char *)s; - last = p; - - if (*p != '\'') /* if string doesn't already begin with "'" */ - *r++ ='\''; /* start putting it in quotes */ - - while (*p) { - last = p; - if (*p == '\'') { /* turn each ' into ''\' */ - if (p != s) /* except if it's the first ', in which case */ - *r++ = '\''; /* only escape it */ - *r++ = '\\'; - *r++ = '\''; - while (*++p && *p == '\'') { /* keep turning each consecutive 's into \' */ - *r++ = '\\'; - *r++ = '\''; - } - if (*p) /* if more input follows, terminate */ - *r++ = '\''; /* what we have so far */ - } else { - *r++ = *p++; - } - } - - if (*last != '\'') /* if the last one isn't already a ' */ - *r++ = '\''; /* terminate the whole shebang */ - - *r = '\0'; - - return ret; /* technically, we lose (but not leak) a couple of */ - /* bytes (twice the number of consecutive 's in the */ - /* input or so), but since these are relatively rare */ - /* and short-lived strings, not sure if a trip to */ - /* wstrdup+wfree worths the gain. */ -} diff --git a/WINGs/wapplication.c b/WINGs/wapplication.c index 07164ec0..45070a22 100644 --- a/WINGs/wapplication.c +++ b/WINGs/wapplication.c @@ -101,21 +101,21 @@ static char *checkFile(const char *path, const char *folder, const char *ext, co slen = strlen(path) + strlen(resource) + 1 + extralen; ret = wmalloc(slen); - if (wstrlcpy(ret, path, slen) >= slen) + if (strlcpy(ret, path, slen) >= slen) goto error; if (folder && - (wstrlcat(ret, "/", slen) >= slen || - wstrlcat(ret, folder, slen) >= slen)) + (strlcat(ret, "/", slen) >= slen || + strlcat(ret, folder, slen) >= slen)) goto error; if (ext && - (wstrlcat(ret, "/", slen) >= slen || - wstrlcat(ret, ext, slen) >= slen)) + (strlcat(ret, "/", slen) >= slen || + strlcat(ret, ext, slen) >= slen)) goto error; - if (wstrlcat(ret, "/", slen) >= slen || - wstrlcat(ret, resource, slen) >= slen) + if (strlcat(ret, "/", slen) >= slen || + strlcat(ret, resource, slen) >= slen) goto error; if (access(ret, F_OK) != 0) diff --git a/WINGs/wbrowser.c b/WINGs/wbrowser.c index 4465b73c..b25bc8c1 100644 --- a/WINGs/wbrowser.c +++ b/WINGs/wbrowser.c @@ -720,14 +720,14 @@ char *WMGetBrowserPathToColumn(WMBrowser * bPtr, int column) path = wmalloc(slen); /* ignore first `/' */ for (i = 0; i <= column; i++) { - if (wstrlcat(path, bPtr->pathSeparator, slen) >= slen) + if (strlcat(path, bPtr->pathSeparator, slen) >= slen) goto error; item = WMGetListSelectedItem(bPtr->columns[i]); if (!item) break; - if (wstrlcat(path, item->text, slen) >= slen) + if (strlcat(path, item->text, slen) >= slen) goto error; } @@ -782,7 +782,7 @@ WMArray *WMGetBrowserPaths(WMBrowser * bPtr) path = wmalloc(slen); /* ignore first `/' */ for (i = 0; i <= column; i++) { - wstrlcat(path, bPtr->pathSeparator, slen); + strlcat(path, bPtr->pathSeparator, slen); if (i == column) { item = lastItem; } else { @@ -790,7 +790,7 @@ WMArray *WMGetBrowserPaths(WMBrowser * bPtr) } if (!item) break; - wstrlcat(path, item->text, slen); + strlcat(path, item->text, slen); } WMAddToArray(paths, path); } @@ -1130,25 +1130,25 @@ static char *createTruncatedString(WMFont * font, const char *text, int *textLen if (width >= 3 * dLen) { int tmpTextLen = *textLen; - if (wstrlcpy(textBuf, text, slen) >= slen) + if (strlcpy(textBuf, text, slen) >= slen) goto error; while (tmpTextLen && (WMWidthOfString(font, textBuf, tmpTextLen) + 3 * dLen > width)) tmpTextLen--; - if (wstrlcpy(textBuf + tmpTextLen, "...", slen) >= slen) + if (strlcpy(textBuf + tmpTextLen, "...", slen) >= slen) goto error; *textLen = tmpTextLen + 3; } else if (width >= 2 * dLen) { - if (wstrlcpy(textBuf, "..", slen) >= slen) + if (strlcpy(textBuf, "..", slen) >= slen) goto error; *textLen = 2; } else if (width >= dLen) { - if (wstrlcpy(textBuf, ".", slen) >= slen) + if (strlcpy(textBuf, ".", slen) >= slen) goto error; *textLen = 1; diff --git a/WINGs/wfilepanel.c b/WINGs/wfilepanel.c index 1da05355..cb01ca1a 100644 --- a/WINGs/wfilepanel.c +++ b/WINGs/wfilepanel.c @@ -513,12 +513,12 @@ static void listDirectoryOnColumn(WMFilePanel * panel, int column, const char *p if (strcmp(dentry->d_name, ".") == 0 || strcmp(dentry->d_name, "..") == 0) continue; - if (wstrlcpy(pbuf, path, sizeof(pbuf)) >= sizeof(pbuf)) + if (strlcpy(pbuf, path, sizeof(pbuf)) >= sizeof(pbuf)) goto out; if (strcmp(path, "/") != 0 && - wstrlcat(pbuf, "/", sizeof(pbuf)) >= sizeof(pbuf)) + strlcat(pbuf, "/", sizeof(pbuf)) >= sizeof(pbuf)) goto out; - if (wstrlcat(pbuf, dentry->d_name, sizeof(pbuf)) >= sizeof(pbuf)) + if (strlcat(pbuf, dentry->d_name, sizeof(pbuf)) >= sizeof(pbuf)) goto out; if (stat(pbuf, &stat_buf) != 0) { @@ -626,11 +626,11 @@ static void createDir(WMWidget *widget, void *p_panel) file = wmalloc(slen); if (directory && - (wstrlcat(file, directory, slen) >= slen || - wstrlcat(file, "/", slen) >= slen)) + (strlcat(file, directory, slen) >= slen || + strlcat(file, "/", slen) >= slen)) goto out; - if (wstrlcat(file, dirName, slen) >= slen) + if (strlcat(file, dirName, slen) >= slen) goto out; if (mkdir(file, 00777) != 0) { diff --git a/WINGs/wfontpanel.c b/WINGs/wfontpanel.c index c57678ab..884f6ce6 100644 --- a/WINGs/wfontpanel.c +++ b/WINGs/wfontpanel.c @@ -558,7 +558,7 @@ static void listFamilies(WMScreen * scr, WMFontPanel * panel) WMListItem *item; WM_ITERATE_ARRAY(array, fam, i) { - wstrlcpy(buffer, fam->name, sizeof(buffer)); + strlcpy(buffer, fam->name, sizeof(buffer)); item = WMAddListItem(panel->famLs, buffer); item->clientData = fam; @@ -640,7 +640,7 @@ static void familyClick(WMWidget * w, void *data) int top = 0; WMListItem *fitem; - wstrlcpy(buffer, face->typeface, sizeof(buffer)); + strlcpy(buffer, face->typeface, sizeof(buffer)); if (strcasecmp(face->typeface, "Roman") == 0) top = 1; if (strcasecmp(face->typeface, "Regular") == 0) @@ -773,7 +773,7 @@ static void setFontPanelFontName(FontPanel * panel, const char *family, const ch int top = 0; WMListItem *fitem; - wstrlcpy(buffer, face->typeface, sizeof(buffer)); + strlcpy(buffer, face->typeface, sizeof(buffer)); if (strcasecmp(face->typeface, "Roman") == 0) top = 1; if (top) diff --git a/WINGs/wtextfield.c b/WINGs/wtextfield.c index feb1d189..f8cf253e 100644 --- a/WINGs/wtextfield.c +++ b/WINGs/wtextfield.c @@ -404,7 +404,7 @@ void WMInsertTextFieldText(WMTextField * tPtr, const char *text, int position) if (position < 0 || position >= tPtr->textLen) { /* append the text at the end */ - wstrlcat(tPtr->text, text, tPtr->bufferSize); + strlcat(tPtr->text, text, tPtr->bufferSize); tPtr->textLen += len; tPtr->cursorPosition += len; incrToFit(tPtr); @@ -473,7 +473,7 @@ void WMSetTextFieldText(WMTextField * tPtr, const char *text) tPtr->bufferSize = tPtr->textLen + TEXT_BUFFER_INCR; tPtr->text = wrealloc(tPtr->text, tPtr->bufferSize); } - wstrlcpy(tPtr->text, text, tPtr->bufferSize); + strlcpy(tPtr->text, text, tPtr->bufferSize); } tPtr->cursorPosition = tPtr->selection.position = tPtr->textLen; diff --git a/WPrefs.app/KeyboardShortcuts.c b/WPrefs.app/KeyboardShortcuts.c index 8df69a9e..49d94ab7 100644 --- a/WPrefs.app/KeyboardShortcuts.c +++ b/WPrefs.app/KeyboardShortcuts.c @@ -365,7 +365,7 @@ char *capture_shortcut(Display *dpy, Bool *capturing, Bool convert_case) if ((numlock_mask != Mod5Mask) && (ev.xkey.state & Mod5Mask)) strcat(buffer, "Mod5+"); - wstrlcat(buffer, key, sizeof(buffer)); + strlcat(buffer, key, sizeof(buffer)); return wstrdup(buffer); } diff --git a/configure.ac b/configure.ac index 78313976..72222d7d 100644 --- a/configure.ac +++ b/configure.ac @@ -391,39 +391,6 @@ dnl the flag 'O_NOFOLLOW' for 'open' is used in WINGs WM_FUNC_OPEN_NOFOLLOW -dnl Check for strlcat/strlcpy -dnl ========================= -m4_divert_push([INIT_PREPARE])dnl -AC_ARG_WITH([libbsd], - [AS_HELP_STRING([--without-libbsd], [do not use libbsd for strlcat and strlcpy [default=check]])], - [AS_IF([test "x$with_libbsd" != "xno"], - [with_libbsd=bsd], - [with_libbsd=] - )], - [with_libbsd=bsd]) -m4_divert_pop([INIT_PREPARE])dnl - -tmp_libs=$LIBS -AC_SEARCH_LIBS([strlcat],[$with_libbsd], - [AC_DEFINE(HAVE_STRLCAT, 1, [Define if strlcat is available])], - [], - [] -) -AC_SEARCH_LIBS([strlcpy],[$with_libbsd], - [AC_DEFINE(HAVE_STRLCAT, 1, [Define if strlcpy is available])], - [], - [] -) -LIBS=$tmp_libs - -LIBBSD= -AS_IF([test "x$ac_cv_search_strlcat" = "x-lbsd" -o "x$ac_cv_search_strlcpy" = "x-lbsd"], - [LIBBSD=-lbsd - AC_CHECK_HEADERS([bsd/string.h])] -) -AC_SUBST(LIBBSD) - - dnl Check for OpenBSD kernel memory interface - kvm(3) dnl ================================================== AS_IF([test "x$WM_OSDEP" = "xbsd"], diff --git a/src/appmenu.c b/src/appmenu.c index 11191641..fcc53ac9 100644 --- a/src/appmenu.c +++ b/src/appmenu.c @@ -94,7 +94,7 @@ static WMenu *parseMenuCommand(WScreen * scr, Window win, char **slist, int coun wwarning(_("appmenu: bad menu entry \"%s\" in window %lx"), slist[*index], win); return NULL; } - if (wstrlcpy(title, &slist[*index][pos], sizeof(title)) >= sizeof(title)) { + if (strlcpy(title, &slist[*index][pos], sizeof(title)) >= sizeof(title)) { wwarning(_("appmenu: menu command size exceeded in window %lx"), win); return NULL; } @@ -127,7 +127,7 @@ static WMenu *parseMenuCommand(WScreen * scr, Window win, char **slist, int coun slist[*index], win); return NULL; } - wstrlcpy(title, &slist[*index][pos], sizeof(title)); + strlcpy(title, &slist[*index][pos], sizeof(title)); rtext[0] = 0; } else { if (sscanf(slist[*index], "%i %i %i %i %s %n", @@ -137,7 +137,7 @@ static WMenu *parseMenuCommand(WScreen * scr, Window win, char **slist, int coun slist[*index], win); return NULL; } - wstrlcpy(title, &slist[*index][pos], sizeof(title)); + strlcpy(title, &slist[*index][pos], sizeof(title)); } data = wmalloc(sizeof(WAppMenuData)); if (data == NULL) { @@ -174,7 +174,7 @@ static WMenu *parseMenuCommand(WScreen * scr, Window win, char **slist, int coun return NULL; } - wstrlcpy(title, &slist[*index][pos], sizeof(title)); + strlcpy(title, &slist[*index][pos], sizeof(title)); *index += 1; submenu = parseMenuCommand(scr, win, slist, count, index); diff --git a/src/defaults.c b/src/defaults.c index 1532692a..e4ef4a21 100644 --- a/src/defaults.c +++ b/src/defaults.c @@ -2205,7 +2205,7 @@ static int getKeybind(WScreen * scr, WDefaultEntry * entry, WMPropList * value, return True; } - wstrlcpy(buf, val, MAX_SHORTCUT_LENGTH); + strlcpy(buf, val, MAX_SHORTCUT_LENGTH); b = (char *)buf; diff --git a/src/dialog.c b/src/dialog.c index e1aad51a..7b0298f5 100644 --- a/src/dialog.c +++ b/src/dialog.c @@ -605,9 +605,9 @@ static void listPixmaps(WScreen *scr, WMList *lPtr, const char *path) if (strcmp(dentry->d_name, ".") == 0 || strcmp(dentry->d_name, "..") == 0) continue; - if (wstrlcpy(pbuf, apath, sizeof(pbuf)) >= sizeof(pbuf) || - wstrlcat(pbuf, "/", sizeof(pbuf)) >= sizeof(pbuf) || - wstrlcat(pbuf, dentry->d_name, sizeof(pbuf)) >= sizeof(pbuf)) { + if (strlcpy(pbuf, apath, sizeof(pbuf)) >= sizeof(pbuf) || + strlcat(pbuf, "/", sizeof(pbuf)) >= sizeof(pbuf) || + strlcat(pbuf, dentry->d_name, sizeof(pbuf)) >= sizeof(pbuf)) { wwarning(_("full path for file \"%s\" in \"%s\" is longer than %d bytes, skipped"), dentry->d_name, path, (int) (sizeof(pbuf) - 1) ); continue; diff --git a/src/rootmenu.c b/src/rootmenu.c index c7931580..bac0ce10 100644 --- a/src/rootmenu.c +++ b/src/rootmenu.c @@ -406,7 +406,7 @@ static Bool addShortcut(const char *file, const char *shortcutDefinition, WMenu ptr = wmalloc(sizeof(Shortcut)); - wstrlcpy(buf, shortcutDefinition, MAX_SHORTCUT_LENGTH); + strlcpy(buf, shortcutDefinition, MAX_SHORTCUT_LENGTH); b = (char *)buf; /* get modifiers */ diff --git a/src/workspace.c b/src/workspace.c index 6686c1a1..2e4020ac 100644 --- a/src/workspace.c +++ b/src/workspace.c @@ -797,7 +797,7 @@ void wWorkspaceMenuUpdate(WScreen * scr, WMenu * menu) i = scr->workspace_count - (menu->entry_no - MC_WORKSPACE1); ws = menu->entry_no - MC_WORKSPACE1; while (i > 0) { - wstrlcpy(title, scr->workspaces[ws]->name, MAX_WORKSPACENAME_WIDTH); + strlcpy(title, scr->workspaces[ws]->name, MAX_WORKSPACENAME_WIDTH); entry = wMenuAddCallback(menu, title, switchWSCommand, (void *)ws); entry->flags.indicator = 1; -- 2.39.5 From 72a1f8cb9ebb0e4e66eeab144028f62d7ce01820 Mon Sep 17 00:00:00 2001 From: Stu Black Date: Sun, 26 Oct 2025 10:10:19 -0400 Subject: [PATCH 02/24] Throw some comments around the FST table for tokenizing command line. This is slated for replacement, but it will help to be better documented first. --- WINGs/string.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/WINGs/string.c b/WINGs/string.c index 4851b6f7..527cf4d5 100644 --- a/WINGs/string.c +++ b/WINGs/string.c @@ -31,15 +31,16 @@ typedef struct { } DFA; static DFA mtable[9][6] = { - {{3, 1}, {0, 0}, {4, 0}, {1, 0}, {8, 0}, {6, 0}}, - {{1, 1}, {1, 1}, {2, 0}, {3, 0}, {5, 0}, {1, 1}}, - {{1, 1}, {1, 1}, {1, 1}, {1, 1}, {5, 0}, {1, 1}}, - {{3, 1}, {5, 0}, {4, 0}, {1, 0}, {5, 0}, {6, 0}}, - {{3, 1}, {3, 1}, {3, 1}, {3, 1}, {5, 0}, {3, 1}}, - {{-1, -1}, {0, 0}, {0, 0}, {0, 0}, {0, 0}, {0, 0}}, /* final state */ - {{6, 1}, {6, 1}, {7, 0}, {6, 1}, {5, 0}, {3, 0}}, - {{6, 1}, {6, 1}, {6, 1}, {6, 1}, {5, 0}, {6, 1}}, - {{-1, -1}, {0, 0}, {0, 0}, {0, 0}, {0, 0}, {0, 0}}, /* final state */ + /* Ctype: ALPHA BLANK ESCAPE DQUOTE EOS SQUOTE + /* State 0: Start */ {{3, 1}, {0, 0}, {4, 0}, {1, 0}, {8, 0}, {6, 0}}, + /* State 1: In dquote */ {{1, 1}, {1, 1}, {2, 0}, {3, 0}, {5, 0}, {1, 1}}, + /* State 2: Escape in dquote */ {{1, 1}, {1, 1}, {1, 1}, {1, 1}, {5, 0}, {1, 1}}, + /* State 3: In token */ {{3, 1}, {5, 0}, {4, 0}, {1, 0}, {5, 0}, {6, 0}}, + /* State 4: Escaping */ {{3, 1}, {3, 1}, {3, 1}, {3, 1}, {5, 0}, {3, 1}}, + /* State 5: End of string */ {{-1, -1}, {0, 0}, {0, 0}, {0, 0}, {0, 0}, {0, 0}}, /* final state */ + /* State 6: In squote */ {{6, 1}, {6, 1}, {7, 0}, {6, 1}, {5, 0}, {3, 0}}, + /* State 7: Escape in squote */ {{6, 1}, {6, 1}, {6, 1}, {6, 1}, {5, 0}, {6, 1}}, + /* State 8: End of string */ {{-1, -1}, {0, 0}, {0, 0}, {0, 0}, {0, 0}, {0, 0}}, /* final state */ }; char *wtokennext(char *word, char **next) -- 2.39.5 From a7a44397a4a7530499f6b5b30a4893edad344e1d Mon Sep 17 00:00:00 2001 From: Stu Black Date: Mon, 27 Oct 2025 22:42:37 -0400 Subject: [PATCH 03/24] Rewrite all functions from WUtils string.c in Rust. These functions should be gotten rid of as we transition to Rust, but replacing them with minimally tested Rust code should suffice as a first step. --- WINGs/Makefile.am | 1 - WINGs/WINGs/WUtil.h | 2 - WINGs/string.c | 240 ------------------------- wutil-rs/src/lib.rs | 1 + wutil-rs/src/string.rs | 400 +++++++++++++++++++++++++++++++++++++++++ 5 files changed, 401 insertions(+), 243 deletions(-) delete mode 100644 WINGs/string.c create mode 100644 wutil-rs/src/string.rs diff --git a/WINGs/Makefile.am b/WINGs/Makefile.am index 903caad7..0423a8be 100644 --- a/WINGs/Makefile.am +++ b/WINGs/Makefile.am @@ -76,7 +76,6 @@ libWUtil_la_SOURCES = \ misc.c \ notification.c \ proplist.c \ - string.c \ tree.c \ userdefaults.c \ userdefaults.h \ diff --git a/WINGs/WINGs/WUtil.h b/WINGs/WINGs/WUtil.h index 25296206..0caa7882 100644 --- a/WINGs/WINGs/WUtil.h +++ b/WINGs/WINGs/WUtil.h @@ -274,8 +274,6 @@ char* wstrappend(char *dst, const char *src); void wtokensplit(char *command, char ***argv, int *argc); -char* wtokennext(char *word, char **next); - char* wtokenjoin(char **list, int count); void wtokenfree(char **tokens, int count); diff --git a/WINGs/string.c b/WINGs/string.c deleted file mode 100644 index 527cf4d5..00000000 --- a/WINGs/string.c +++ /dev/null @@ -1,240 +0,0 @@ -/* - * Until FreeBSD gets their act together; - * http://www.mail-archive.com/freebsd-hackers@freebsd.org/msg69469.html - */ -#if defined( FREEBSD ) -# undef _XOPEN_SOURCE -#endif - -#include "wconfig.h" - -#include -#include -#include -#include -#ifdef HAVE_BSD_STRING_H -#include -#endif - -#include "WUtil.h" - -#define PRC_ALPHA 0 -#define PRC_BLANK 1 -#define PRC_ESCAPE 2 -#define PRC_DQUOTE 3 -#define PRC_EOS 4 -#define PRC_SQUOTE 5 - -typedef struct { - short nstate; - short output; -} DFA; - -static DFA mtable[9][6] = { - /* Ctype: ALPHA BLANK ESCAPE DQUOTE EOS SQUOTE - /* State 0: Start */ {{3, 1}, {0, 0}, {4, 0}, {1, 0}, {8, 0}, {6, 0}}, - /* State 1: In dquote */ {{1, 1}, {1, 1}, {2, 0}, {3, 0}, {5, 0}, {1, 1}}, - /* State 2: Escape in dquote */ {{1, 1}, {1, 1}, {1, 1}, {1, 1}, {5, 0}, {1, 1}}, - /* State 3: In token */ {{3, 1}, {5, 0}, {4, 0}, {1, 0}, {5, 0}, {6, 0}}, - /* State 4: Escaping */ {{3, 1}, {3, 1}, {3, 1}, {3, 1}, {5, 0}, {3, 1}}, - /* State 5: End of string */ {{-1, -1}, {0, 0}, {0, 0}, {0, 0}, {0, 0}, {0, 0}}, /* final state */ - /* State 6: In squote */ {{6, 1}, {6, 1}, {7, 0}, {6, 1}, {5, 0}, {3, 0}}, - /* State 7: Escape in squote */ {{6, 1}, {6, 1}, {6, 1}, {6, 1}, {5, 0}, {6, 1}}, - /* State 8: End of string */ {{-1, -1}, {0, 0}, {0, 0}, {0, 0}, {0, 0}, {0, 0}}, /* final state */ -}; - -char *wtokennext(char *word, char **next) -{ - char *ptr; - char *ret, *t; - int state, ctype; - - t = ret = wmalloc(strlen(word) + 1); - ptr = word; - - state = 0; - while (1) { - if (*ptr == 0) - ctype = PRC_EOS; - else if (*ptr == '\\') - ctype = PRC_ESCAPE; - else if (*ptr == '"') - ctype = PRC_DQUOTE; - else if (*ptr == '\'') - ctype = PRC_SQUOTE; - else if (*ptr == ' ' || *ptr == '\t') - ctype = PRC_BLANK; - else - ctype = PRC_ALPHA; - - if (mtable[state][ctype].output) { - *t = *ptr; - t++; - *t = 0; - } - state = mtable[state][ctype].nstate; - ptr++; - if (mtable[state][0].output < 0) { - break; - } - } - - if (*ret == 0) { - wfree(ret); - ret = NULL; - } - - if (ctype == PRC_EOS) - *next = NULL; - else - *next = ptr; - - return ret; -} - -/* separate a string in tokens, taking " and ' into account */ -void wtokensplit(char *command, char ***argv, int *argc) -{ - char *token, *line; - int count; - - count = 0; - line = command; - do { - token = wtokennext(line, &line); - if (token) { - if (count == 0) - *argv = wmalloc(sizeof(**argv)); - else - *argv = wrealloc(*argv, (count + 1) * sizeof(**argv)); - (*argv)[count++] = token; - } - } while (token != NULL && line != NULL); - - *argc = count; -} - -char *wtokenjoin(char **list, int count) -{ - int i, j; - char *flat_string, *wspace; - - j = 0; - for (i = 0; i < count; i++) { - if (list[i] != NULL && list[i][0] != 0) { - j += strlen(list[i]); - if (strpbrk(list[i], " \t")) - j += 2; - } - } - - flat_string = wmalloc(j + count + 1); - - for (i = 0; i < count; i++) { - if (list[i] != NULL && list[i][0] != 0) { - if (i > 0 && - strlcat(flat_string, " ", j + count + 1) >= j + count + 1) - goto error; - - wspace = strpbrk(list[i], " \t"); - - if (wspace && - strlcat(flat_string, "\"", j + count + 1) >= j + count + 1) - goto error; - - if (strlcat(flat_string, list[i], j + count + 1) >= j + count + 1) - goto error; - - if (wspace && - strlcat(flat_string, "\"", j + count + 1) >= j + count + 1) - goto error; - } - } - - return flat_string; - -error: - wfree(flat_string); - - return NULL; -} - -void wtokenfree(char **tokens, int count) -{ - while (count--) - wfree(tokens[count]); - wfree(tokens); -} - -char *wtrimspace(const char *s) -{ - const char *t; - - if (s == NULL) - return NULL; - - while (isspace(*s) && *s) - s++; - t = s + strlen(s) - 1; - while (t > s && isspace(*t)) - t--; - - return wstrndup(s, t - s + 1); -} - -char *wstrdup(const char *str) -{ - assert(str != NULL); - - return strcpy(wmalloc(strlen(str) + 1), str); -} - -char *wstrndup(const char *str, size_t len) -{ - char *copy; - - assert(str != NULL); - - len = WMIN(len, strlen(str)); - copy = strncpy(wmalloc(len + 1), str, len); - copy[len] = 0; - - return copy; -} - -char *wstrconcat(const char *str1, const char *str2) -{ - char *str; - size_t slen, slen1; - - if (!str1 && str2) - return wstrdup(str2); - else if (str1 && !str2) - return wstrdup(str1); - else if (!str1 && !str2) - return NULL; - - slen1 = strlen(str1); - slen = slen1 + strlen(str2) + 1; - str = wmalloc(slen); - strcpy(str, str1); - strcpy(str + slen1, str2); - - return str; -} - -char *wstrappend(char *dst, const char *src) -{ - size_t slen; - - if (!src || *src == 0) - return dst; - else if (!dst) - return wstrdup(src); - - slen = strlen(dst) + strlen(src) + 1; - dst = wrealloc(dst, slen); - strcat(dst, src); - - return dst; -} diff --git a/wutil-rs/src/lib.rs b/wutil-rs/src/lib.rs index 68a17ca2..55adc6f7 100644 --- a/wutil-rs/src/lib.rs +++ b/wutil-rs/src/lib.rs @@ -5,3 +5,4 @@ pub mod find_file; pub mod hash_table; pub mod memory; pub mod prop_list; +pub mod string; diff --git a/wutil-rs/src/string.rs b/wutil-rs/src/string.rs new file mode 100644 index 00000000..c1778ea4 --- /dev/null +++ b/wutil-rs/src/string.rs @@ -0,0 +1,400 @@ +//! String manipulation utilities. +//! +//! ## Rust rewrite notes +//! +//! These are more or less bug-for-bug reimplementations of the original WUtils +//! functions. Avoid using these functions in new code. + +use std::{ + ffi::{c_char, c_int, CStr}, + iter, mem, ptr, slice, +}; + +use crate::memory::{alloc_bytes, alloc_string, ffi::wrealloc, free_bytes}; + +/// Returns a `wmalloc`-managed C-style string that duplicates `s`, which cannot +/// be null. +#[unsafe(no_mangle)] +pub unsafe extern "C" fn wstrdup(s: *const c_char) -> *mut c_char { + assert!(!s.is_null()); + alloc_string(unsafe { CStr::from_ptr(s) }) +} + +/// Returns a `wmalloc`-managed C-style string of the first `n` bytes of +/// `s`, which cannot be null. +/// +/// If `n` exceeds the length of `s`, uses the lesser value. +#[unsafe(no_mangle)] +pub unsafe extern "C" fn wstrndup(s: *const c_char, len: usize) -> *mut c_char { + assert!(!s.is_null()); + let s = unsafe { CStr::from_ptr(s) }; + let len = usize::min(len, s.count_bytes()) + 1; + let copy: *mut c_char = alloc_bytes(len + 1).cast(); // Implicitly zeroed. + unsafe { + ptr::copy_nonoverlapping(s.as_ptr(), copy, len); + } + copy.cast::() +} + +/// Concatenates `s1` and `s2` into a `wmalloc`-managed C-style string. +#[unsafe(no_mangle)] +pub unsafe extern "C" fn wstrconcat(s1: *const c_char, s2: *const c_char) -> *mut c_char { + match (s1.is_null(), s2.is_null()) { + (true, true) => ptr::null_mut(), + (true, false) => unsafe { wstrdup(s1) }, + (false, true) => unsafe { wstrdup(s2) }, + (false, false) => unsafe { + let s1 = CStr::from_ptr(s1); + let l1 = s1.count_bytes(); + let s2 = CStr::from_ptr(s2); + let l2 = s2.count_bytes(); + let s: *mut c_char = alloc_bytes(l1 + l2 + 1).cast(); // Implicitly zeroed. + ptr::copy_nonoverlapping(s1.as_ptr(), s, l1); + ptr::copy_nonoverlapping(s2.as_ptr(), s.offset(l1 as isize), l2); + s + }, + } +} + +/// Appends `src` to `dest`, destructively reallocating `dest` and returning a +/// new `wmalloc`-managed pointer where the result is stored. `dst` must come +/// from `wmalloc` or `wrealloc`. +#[unsafe(no_mangle)] +pub unsafe extern "C" fn wstrappend(dst: *mut c_char, src: *const c_char) -> *mut c_char { + if src.is_null() { + return dst; + } + let src = unsafe { CStr::from_ptr(src) }; + let src_len = src.count_bytes(); + if src_len == 0 { + return dst; + } + + if dst.is_null() { + return unsafe { wstrdup(src.as_ptr()) }; + } + let dst_len = unsafe { CStr::from_ptr(dst).count_bytes() + 1 }; + + let len = dst_len + src_len + 1; + let result: *mut c_char = unsafe { wrealloc(dst.cast(), len).cast() }; + unsafe { + ptr::copy_nonoverlapping( + src.as_ptr(), + result.offset(dst_len.try_into().unwrap()), + src_len, + ); + } + result +} + +/// Strips leading and trailing whitespace from `s`, returning a +/// `wmalloc`-managed C-style string holding the result. +/// +/// ## Rust rewite notes +/// +/// This uses a slightly different notion of "space character" than the original +/// C implementation did. `s` must be valid UTF-8. +#[unsafe(no_mangle)] +pub unsafe extern "C" fn wtrimspace(s: *const c_char) -> *mut c_char { + if s.is_null() { + return ptr::null_mut(); + } + + let Ok(s) = (unsafe { CStr::from_ptr(s).to_str() }) else { + // TODO: complain. + return ptr::null_mut(); + }; + let s: Vec<_> = s.trim().bytes().chain(iter::once(b'\0')).collect(); + + alloc_string(unsafe { CStr::from_bytes_with_nul_unchecked(&s) }) +} + +/// Splits `command` into tokens with approximately the same rules as used in +/// the shell for splitting up a command into program arguments. +fn tokensplit(command: &[u8]) -> Vec> { + enum Mode { + Start, + Unquoted(Vec), + Escape(Context), + Quoted { token: Vec, delimiter: u8, }, + } + enum Context { + Unquoted(Vec), + Quoted { token: Vec, delimiter: u8, }, + } + let mut out = Vec::new(); + let mut mode = Mode::Start; + for &b in command { + mode = match (mode, b) { + (Mode::Start, b'\\') => Mode::Escape(Context::Unquoted(vec![])), + (Mode::Start, b'\'' | b'"') => Mode::Quoted { token: vec![], delimiter: b }, + (Mode::Start, b' ' | b'\t') => Mode::Start, + (Mode::Start, _) => Mode::Unquoted(vec![b]), + (Mode::Unquoted(token), b'\\') => Mode::Escape(Context::Unquoted(token)), + (Mode::Unquoted(token), b'\'' | b'"') => Mode::Quoted { token, delimiter: b }, + (Mode::Unquoted(token), b' ' | b'\t') => { + out.push(token); + Mode::Start + } + (Mode::Unquoted(mut token), _) => { + token.push(b); + Mode::Unquoted(token) + } + (Mode::Escape(Context::Unquoted(mut token)), _) => { + token.push(b); + Mode::Unquoted(token) + } + (Mode::Escape(Context::Quoted { mut token, delimiter }), _) => { + token.push(b); + Mode::Quoted { token, delimiter } + } + (Mode::Quoted { token, delimiter }, _) if b == delimiter => { + Mode::Unquoted(token) + } + (Mode::Quoted { token, delimiter }, _) if b == b'\\' => { + Mode::Escape(Context::Quoted { token, delimiter }) + } + (Mode::Quoted { mut token, delimiter }, _) => { + token.push(b); + Mode::Quoted { token, delimiter } + } + } + } + match mode { + Mode::Start => (), + Mode::Unquoted(token) => out.push(token), + Mode::Quoted { token, .. } => out.push(token), + Mode::Escape(Context::Unquoted(token)) => out.push(token), + Mode::Escape(Context::Quoted { token, .. }) => out.push(token), + } + out +} + +/// Splits `command` into tokens, storing the number of tokens in `argc` and +/// `wmalloc`-managed C-style strings for each token in `argv`. Call +/// [`wtokenfree`] to free `argv`. +#[unsafe(no_mangle)] +pub unsafe extern "C" fn wtokensplit( + command: *const c_char, + argv: *mut *mut *mut c_char, + argc: *mut c_int, +) { + if argv.is_null() || argc.is_null() { + return; + } + unsafe { + *argv = ptr::null_mut(); + *argc = 0; + } + if command.is_null() { + return; + } + let command = unsafe { CStr::from_ptr(command) }; + let Ok(command) = command.to_str() else { + return; + }; + let tokens = tokensplit(command.as_bytes()); + if tokens.is_empty() { + return; + } + + let argv = unsafe { + *argv = alloc_bytes(mem::size_of::<*mut c_char>() * tokens.len()).cast::<*mut c_char>(); + *argc = tokens.len() as c_int; + slice::from_raw_parts_mut(*argv, tokens.len()) + }; + for (dest, mut token) in argv.iter_mut().zip(tokens.into_iter()) { + token.push(b'\0'); + *dest = alloc_string(unsafe { CStr::from_bytes_with_nul_unchecked(&token) }); + } +} + +/// Frees an `argv` populated by [`wtokensplit`]. +#[unsafe(no_mangle)] +pub unsafe extern "C" fn wtokenfree(tokens: *mut *mut c_char, count: c_int) { + if tokens.is_null() { + return; + } + if count > 0 { + let tokens = unsafe { slice::from_raw_parts_mut(tokens, count as usize) }; + for token in tokens { + unsafe { + free_bytes(*token); + } + } + } + unsafe { + free_bytes(tokens.cast::()); + } +} + +/// Joins the tokens of the `count` elements of `list` into a single string, +/// enclosing them in double quotes (`"`) if necessary. Returns a +/// `wmalloc`-managed C-style string with the result, or null on failure. +/// +/// ## Rust rewrite notes +/// +/// `list` must consist of valid UTF-8 strings. This reimplementation does not +/// attempt to improve on the original, so it has bad failure modes (like not +/// escaping quotes in the input). Do not use this function in new code. +#[unsafe(no_mangle)] +pub unsafe extern "C" fn wtokenjoin(list: *const *const char, count: c_int) -> *mut c_char { + if list.is_null() || count <= 0 { + return ptr::null_mut(); + } + + let list = unsafe { + slice::from_raw_parts(list.cast::<*const u8>(), count as usize) + }; + + let mut buffer = Vec::new(); + for term in list { + if term.is_null() { + continue; + } + let term = unsafe { CStr::from_ptr(*term) }; + if term.is_empty() { + continue; + } + if !buffer.is_empty() { + buffer.push(b' '); + } + let term = term.to_bytes(); + if term.iter().find(|&&x| x == b' ' || x == b'\t').is_some() { + buffer.push(b'"'); + buffer.extend_from_slice(term); + buffer.push(b'"'); + } else { + buffer.extend_from_slice(term); + } + } + buffer.push(b'\0'); + + if let Ok(buffer) = CStr::from_bytes_until_nul(&buffer) { + alloc_string(buffer) + } else { + return ptr::null_mut(); + } +} + +#[cfg(test)] +mod test { + use super::{wtokenfree, wtokensplit}; + + use std::{ffi::CStr, ptr, slice}; + + #[test] + fn split_empty_whitespace() { + let mut argv = ptr::null_mut(); + let mut argc = 0; + + unsafe { wtokensplit(c"".as_ptr(), &mut argv, &mut argc); } + assert_eq!(argc, 0); + assert_eq!(argv, ptr::null_mut()); + unsafe { wtokenfree(argv, argc); } + + unsafe { wtokensplit(c" ".as_ptr(), &mut argv, &mut argc); } + assert_eq!(argc, 0); + assert_eq!(argv, ptr::null_mut()); + unsafe { wtokenfree(argv, argc); } + + unsafe { wtokensplit(c" \t ".as_ptr(), &mut argv, &mut argc); } + assert_eq!(argc, 0); + assert_eq!(argv, ptr::null_mut()); + unsafe { wtokenfree(argv, argc); } + + unsafe { wtokensplit(c" \t ".as_ptr(), &mut argv, &mut argc); } + assert_eq!(argc, 0); + assert_eq!(argv, ptr::null_mut()); + unsafe { wtokenfree(argv, argc); } + + unsafe { wtokensplit(c"\t\t".as_ptr(), &mut argv, &mut argc); } + assert_eq!(argc, 0); + assert_eq!(argv, ptr::null_mut()); + unsafe { wtokenfree(argv, argc); } + } + + fn args_of(argv: *mut *mut u8, argc: usize) -> Vec { + let mut v = Vec::with_capacity(argc); + for s in unsafe { slice::from_raw_parts(argv, argc) } { + if s.is_null() { + return v; + } + v.push(String::from(unsafe { CStr::from_ptr(*s) }.to_str().unwrap())); + } + v + } + + #[test] + fn split_empty_quoted() { + let mut argv = ptr::null_mut(); + let mut argc = 0; + + unsafe { wtokensplit(c"\"\"".as_ptr(), &mut argv, &mut argc); } + assert_eq!(argc, 1); + assert_eq!(args_of(argv, argc as usize), vec![String::from("")]); + unsafe { wtokenfree(argv, argc); } + + argv = ptr::null_mut(); + argc = 0; + + unsafe { wtokensplit(c"''".as_ptr(), &mut argv, &mut argc); } + assert_eq!(argc, 1); + assert_eq!(args_of(argv, argc as usize), vec![String::from("")]); + unsafe { wtokenfree(argv, argc); } + + argv = ptr::null_mut(); + argc = 0; + + unsafe { wtokensplit(c" ''\t".as_ptr(), &mut argv, &mut argc); } + assert_eq!(argc, 1); + assert_eq!(args_of(argv, argc as usize), vec![String::from("")]); + unsafe { wtokenfree(argv, argc); } + } + + #[test] + fn split_one_unquoted() { + let mut argv = ptr::null_mut(); + let mut argc = 0; + + unsafe { wtokensplit(c"hello".as_ptr(), &mut argv, &mut argc); } + assert_eq!(argc, 1); + assert_eq!(args_of(argv, argc as usize), vec![String::from("hello")]); + + unsafe { wtokensplit(c"hello\\\\".as_ptr(), &mut argv, &mut argc); } + assert_eq!(argc, 1); + assert_eq!(args_of(argv, argc as usize), vec![String::from("hello\\")]); + } + + #[test] + fn split_one_quoted() { + let mut argv = ptr::null_mut(); + let mut argc = 0; + + unsafe { wtokensplit(c"\"hello\"".as_ptr(), &mut argv, &mut argc); } + assert_eq!(argc, 1); + assert_eq!(args_of(argv, argc as usize), vec![String::from("hello")]); + + unsafe { wtokensplit(c"\"hello world\"".as_ptr(), &mut argv, &mut argc); } + assert_eq!(argc, 1); + assert_eq!(args_of(argv, argc as usize), vec![String::from("hello world")]); + } + + #[test] + fn split_multi() { + let mut argv = ptr::null_mut(); + let mut argc = 0; + + unsafe { + wtokensplit( + c"\"hello world\" what\\'s happening' here it\\'s weird'".as_ptr(), + &mut argv, + &mut argc, + ); + } + assert_eq!(argc, 3); + assert_eq!(args_of(argv, argc as usize), vec![String::from("hello world"), + String::from("what's"), + String::from("happening here it's weird")]); + } +} -- 2.39.5 From d2046de7ff5734002ae64a949791a1ec6a90085e Mon Sep 17 00:00:00 2001 From: Stu Black Date: Mon, 27 Oct 2025 23:13:15 -0400 Subject: [PATCH 04/24] Unit tests for `wtokenjoin`. --- wutil-rs/src/string.rs | 40 +++++++++++++++++++++++++++++++++++++--- 1 file changed, 37 insertions(+), 3 deletions(-) diff --git a/wutil-rs/src/string.rs b/wutil-rs/src/string.rs index c1778ea4..92492e25 100644 --- a/wutil-rs/src/string.rs +++ b/wutil-rs/src/string.rs @@ -240,7 +240,7 @@ pub unsafe extern "C" fn wtokenfree(tokens: *mut *mut c_char, count: c_int) { #[unsafe(no_mangle)] pub unsafe extern "C" fn wtokenjoin(list: *const *const char, count: c_int) -> *mut c_char { if list.is_null() || count <= 0 { - return ptr::null_mut(); + return alloc_string(c""); } let list = unsafe { @@ -279,9 +279,9 @@ pub unsafe extern "C" fn wtokenjoin(list: *const *const char, count: c_int) -> * #[cfg(test)] mod test { - use super::{wtokenfree, wtokensplit}; + use super::{wtokenfree, wtokensplit, wtokenjoin}; - use std::{ffi::CStr, ptr, slice}; + use std::{ffi::{c_char, c_int, CStr}, ptr, slice}; #[test] fn split_empty_whitespace() { @@ -397,4 +397,38 @@ mod test { String::from("what's"), String::from("happening here it's weird")]); } + + /// Calls `f(wtokenjoin(list))` transparently. + fn with_list(list: &[&CStr], f: impl FnOnce(Option<&CStr>) -> R) -> R { + let list: Vec<_> = list.iter().map(|s| s.as_ptr().cast::()).collect(); + let joined = unsafe { wtokenjoin(list.as_slice().as_ptr().cast(), list.len() as c_int) }; + let result = f(if joined.is_null() { None } else { unsafe { Some(CStr::from_ptr(joined)) } }); + unsafe { crate::memory::free_bytes(joined.cast()); } + result + } + + #[test] + fn join_nothing() { + with_list(&[], |joined| { + assert_eq!(joined.unwrap(), c""); + }); + + with_list(&[c"", c""], |joined| { + assert_eq!(joined.unwrap(), c""); + }); + } + + #[test] + fn join_basic() { + with_list(&[c"hello", c"world"], |joined| { + assert_eq!(joined.unwrap(), c"hello world"); + }); + } + + #[test] + fn join_quoted() { + with_list(&[c"hello", c"there world"], |joined| { + assert_eq!(joined.unwrap(), c"hello \"there world\""); + }); + } } -- 2.39.5 From 927cc93e0ad90e719b27f62df151a401fcf3bb86 Mon Sep 17 00:00:00 2001 From: Stu Black Date: Tue, 28 Oct 2025 21:16:18 -0400 Subject: [PATCH 05/24] Add string.rs to Makefile.am for wutil-rs. Without this, `make` won't automatically rebuild wutil-rs when string.rs changes. --- wutil-rs/Makefile.am | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/wutil-rs/Makefile.am b/wutil-rs/Makefile.am index 00067adb..afbf427a 100644 --- a/wutil-rs/Makefile.am +++ b/wutil-rs/Makefile.am @@ -9,7 +9,8 @@ RUST_SOURCES = \ src/hash_table.rs \ src/lib.rs \ src/memory.rs \ - src/prop_list.rs + src/prop_list.rs \ + src/string.rs RUST_EXTRA = \ Cargo.lock \ -- 2.39.5 From dfd77b11a91be77a711b5cbdea7f5590b530f76a Mon Sep 17 00:00:00 2001 From: Stu Black Date: Wed, 12 Nov 2025 16:46:57 -0500 Subject: [PATCH 06/24] Don't scan ahead unnecessarily in wstrndup. --- wutil-rs/src/string.rs | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/wutil-rs/src/string.rs b/wutil-rs/src/string.rs index 92492e25..892f3f3f 100644 --- a/wutil-rs/src/string.rs +++ b/wutil-rs/src/string.rs @@ -20,18 +20,22 @@ pub unsafe extern "C" fn wstrdup(s: *const c_char) -> *mut c_char { alloc_string(unsafe { CStr::from_ptr(s) }) } -/// Returns a `wmalloc`-managed C-style string of the first `n` bytes of -/// `s`, which cannot be null. +/// Returns a `wmalloc`-managed C-style string of the first `len` bytes of `s`, +/// which cannot be null. /// -/// If `n` exceeds the length of `s`, uses the lesser value. +/// If `len` exceeds the length of `s`, uses the lesser value. #[unsafe(no_mangle)] pub unsafe extern "C" fn wstrndup(s: *const c_char, len: usize) -> *mut c_char { assert!(!s.is_null()); - let s = unsafe { CStr::from_ptr(s) }; - let len = usize::min(len, s.count_bytes()) + 1; + let len = unsafe { + slice::from_raw_parts(s, len) + .into_iter() + .position(|p| *p == 0) + .unwrap_or(len) + }; let copy: *mut c_char = alloc_bytes(len + 1).cast(); // Implicitly zeroed. unsafe { - ptr::copy_nonoverlapping(s.as_ptr(), copy, len); + ptr::copy_nonoverlapping(s, copy, len); } copy.cast::() } -- 2.39.5 From c298b5f96f2c496a18de7c498b03683aa4abd7b0 Mon Sep 17 00:00:00 2001 From: Stu Black Date: Wed, 12 Nov 2025 16:48:47 -0500 Subject: [PATCH 07/24] Avoid an unnecessary allocation in wtrimspace. --- wutil-rs/src/string.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/wutil-rs/src/string.rs b/wutil-rs/src/string.rs index 892f3f3f..98aec8e9 100644 --- a/wutil-rs/src/string.rs +++ b/wutil-rs/src/string.rs @@ -108,9 +108,10 @@ pub unsafe extern "C" fn wtrimspace(s: *const c_char) -> *mut c_char { // TODO: complain. return ptr::null_mut(); }; - let s: Vec<_> = s.trim().bytes().chain(iter::once(b'\0')).collect(); - - alloc_string(unsafe { CStr::from_bytes_with_nul_unchecked(&s) }) + let trimmed = s.trim(); + let ptr = trimmed.as_ptr(); + let len = trimmed.len(); + unsafe { wstrndup(ptr, len) } } /// Splits `command` into tokens with approximately the same rules as used in -- 2.39.5 From e1a263cc5b8d04efd1964b2164d556e9116f191d Mon Sep 17 00:00:00 2001 From: Stu Black Date: Wed, 29 Oct 2025 12:32:32 -0400 Subject: [PATCH 08/24] Prune unused WMTree API functions. This is a first step towards migrating WINGs/tree.c to Rust. Happy finding: no parent pointer are actually needed, so we don't have to use a shared structure for node pointers! --- WINGs/WINGs/WUtil.h | 19 ------------ WINGs/tree.c | 75 --------------------------------------------- 2 files changed, 94 deletions(-) diff --git a/WINGs/WINGs/WUtil.h b/WINGs/WINGs/WUtil.h index 0caa7882..ea1ec114 100644 --- a/WINGs/WINGs/WUtil.h +++ b/WINGs/WINGs/WUtil.h @@ -631,36 +631,17 @@ WMTreeNode* WMInsertNodeInTree(WMTreeNode *parent, int index, WMTreeNode *aNode) #define WMAddNodeToTree(parent, aNode) WMInsertNodeInTree(parent, -1, aNode) -void WMDestroyTreeNode(WMTreeNode *aNode); - -void WMDeleteLeafForTreeNode(WMTreeNode *aNode, int index); - -void WMRemoveLeafForTreeNode(WMTreeNode *aNode, void *leaf); - -void* WMReplaceDataForTreeNode(WMTreeNode *aNode, void *newData); - void* WMGetDataForTreeNode(WMTreeNode *aNode); int WMGetTreeNodeDepth(WMTreeNode *aNode); -WMTreeNode* WMGetParentForTreeNode(WMTreeNode *aNode); - -/* Sort only the leaves of the passed node */ -void WMSortLeavesForTreeNode(WMTreeNode *aNode, WMCompareDataProc *comparer); - /* Sort all tree recursively starting from the passed node */ void WMSortTree(WMTreeNode *aNode, WMCompareDataProc *comparer); -/* Returns the first node which matches node's data with cdata by 'match' */ -WMTreeNode* WMFindInTree(WMTreeNode *aTree, WMMatchDataProc *match, void *cdata); - /* Returns the first node where node's data matches cdata by 'match' and node is * at most `limit' depths down from `aTree'. */ WMTreeNode *WMFindInTreeWithDepthLimit(WMTreeNode * aTree, WMMatchDataProc * match, void *cdata, int limit); -/* Returns first tree node that has data == cdata */ -#define WMGetFirstInTree(aTree, cdata) WMFindInTree(aTree, NULL, cdata) - /* Walk every node of aNode with `walk' */ void WMTreeWalk(WMTreeNode *aNode, WMTreeWalkProc * walk, void *data, Bool DepthFirst); diff --git a/WINGs/tree.c b/WINGs/tree.c index f6646848..b747724a 100644 --- a/WINGs/tree.c +++ b/WINGs/tree.c @@ -12,8 +12,6 @@ typedef struct W_TreeNode { int depth; - struct W_TreeNode *parent; - WMFreeDataProc *destructor; } W_TreeNode; @@ -42,7 +40,6 @@ WMTreeNode *WMCreateTreeNodeWithDestructor(void *data, WMFreeDataProc * destruct aNode = (WMTreeNode *) wmalloc(sizeof(W_TreeNode)); aNode->destructor = destructor; aNode->data = data; - aNode->parent = NULL; aNode->depth = 0; aNode->leaves = NULL; /*aNode->leaves = WMCreateArrayWithDestructor(1, destroyNode); */ @@ -57,7 +54,6 @@ WMTreeNode *WMInsertItemInTree(WMTreeNode * parent, int index, void *item) wassertrv(parent != NULL, NULL); aNode = WMCreateTreeNodeWithDestructor(item, parent->destructor); - aNode->parent = parent; aNode->depth = parent->depth + 1; if (!parent->leaves) { parent->leaves = WMCreateArrayWithDestructor(1, destroyNode); @@ -89,7 +85,6 @@ WMTreeNode *WMInsertNodeInTree(WMTreeNode * parent, int index, WMTreeNode * aNod wassertrv(parent != NULL, NULL); wassertrv(aNode != NULL, NULL); - aNode->parent = parent; updateNodeDepth(aNode, parent->depth + 1); if (!parent->leaves) { parent->leaves = WMCreateArrayWithDestructor(1, destroyNode); @@ -103,55 +98,6 @@ WMTreeNode *WMInsertNodeInTree(WMTreeNode * parent, int index, WMTreeNode * aNod return aNode; } -void WMDestroyTreeNode(WMTreeNode * aNode) -{ - wassertr(aNode != NULL); - - if (aNode->parent && aNode->parent->leaves) { - WMRemoveFromArray(aNode->parent->leaves, aNode); - } else { - destroyNode(aNode); - } -} - -void WMDeleteLeafForTreeNode(WMTreeNode * aNode, int index) -{ - wassertr(aNode != NULL); - wassertr(aNode->leaves != NULL); - - WMDeleteFromArray(aNode->leaves, index); -} - -static int sameData(const void *item, const void *data) -{ - return (((WMTreeNode *) item)->data == data); -} - -void WMRemoveLeafForTreeNode(WMTreeNode * aNode, void *leaf) -{ - int index; - - wassertr(aNode != NULL); - wassertr(aNode->leaves != NULL); - - index = WMFindInArray(aNode->leaves, sameData, leaf); - if (index != WANotFound) { - WMDeleteFromArray(aNode->leaves, index); - } -} - -void *WMReplaceDataForTreeNode(WMTreeNode * aNode, void *newData) -{ - void *old; - - wassertrv(aNode != NULL, NULL); - - old = aNode->data; - aNode->data = newData; - - return old; -} - void *WMGetDataForTreeNode(WMTreeNode * aNode) { return aNode->data; @@ -162,20 +108,6 @@ int WMGetTreeNodeDepth(WMTreeNode * aNode) return aNode->depth; } -WMTreeNode *WMGetParentForTreeNode(WMTreeNode * aNode) -{ - return aNode->parent; -} - -void WMSortLeavesForTreeNode(WMTreeNode * aNode, WMCompareDataProc * comparer) -{ - wassertr(aNode != NULL); - - if (aNode->leaves) { - WMSortArray(aNode->leaves, comparer); - } -} - static void sortLeavesForNode(WMTreeNode * aNode, WMCompareDataProc * comparer) { int i; @@ -218,13 +150,6 @@ static WMTreeNode *findNodeInTree(WMTreeNode * aNode, WMMatchDataProc * match, v return NULL; } -WMTreeNode *WMFindInTree(WMTreeNode * aTree, WMMatchDataProc * match, void *cdata) -{ - wassertrv(aTree != NULL, NULL); - - return findNodeInTree(aTree, match, cdata, -1); -} - WMTreeNode *WMFindInTreeWithDepthLimit(WMTreeNode * aTree, WMMatchDataProc * match, void *cdata, int limit) { wassertrv(aTree != NULL, NULL); -- 2.39.5 From 0a04a4c12e8eec0b8f31b748e90c0346833fcd2f Mon Sep 17 00:00:00 2001 From: Stu Black Date: Wed, 29 Oct 2025 15:13:23 -0400 Subject: [PATCH 09/24] Remove depthFirst parameter from WMTreeWalk. WMTreeWalk is only called once, with depthFirst set to true, so we might as well just hard-code that behavior. Also, this parameter appears to have been misnamed (since the search is always DFS) and should properly be named to indicate that it controls if this is a pre- or post-order traversal. --- WINGs/WINGs/WUtil.h | 2 +- WINGs/tree.c | 10 +++------- util/wmmenugen.c | 2 +- 3 files changed, 5 insertions(+), 9 deletions(-) diff --git a/WINGs/WINGs/WUtil.h b/WINGs/WINGs/WUtil.h index ea1ec114..6ca513a5 100644 --- a/WINGs/WINGs/WUtil.h +++ b/WINGs/WINGs/WUtil.h @@ -643,7 +643,7 @@ void WMSortTree(WMTreeNode *aNode, WMCompareDataProc *comparer); WMTreeNode *WMFindInTreeWithDepthLimit(WMTreeNode * aTree, WMMatchDataProc * match, void *cdata, int limit); /* Walk every node of aNode with `walk' */ -void WMTreeWalk(WMTreeNode *aNode, WMTreeWalkProc * walk, void *data, Bool DepthFirst); +void WMTreeWalk(WMTreeNode *aNode, WMTreeWalkProc * walk, void *data); /* ---[ WINGs/data.c ]---------------------------------------------------- */ diff --git a/WINGs/tree.c b/WINGs/tree.c index b747724a..536e9e92 100644 --- a/WINGs/tree.c +++ b/WINGs/tree.c @@ -158,23 +158,19 @@ WMTreeNode *WMFindInTreeWithDepthLimit(WMTreeNode * aTree, WMMatchDataProc * mat return findNodeInTree(aTree, match, cdata, limit); } -void WMTreeWalk(WMTreeNode * aNode, WMTreeWalkProc * walk, void *data, Bool DepthFirst) +void WMTreeWalk(WMTreeNode * aNode, WMTreeWalkProc * walk, void *data) { int i; WMTreeNode *leaf; wassertr(aNode != NULL); - if (DepthFirst) - (*walk)(aNode, data); + (*walk)(aNode, data); if (aNode->leaves) { for (i = 0; i < WMGetArrayItemCount(aNode->leaves); i++) { leaf = (WMTreeNode *)WMGetFromArray(aNode->leaves, i); - WMTreeWalk(leaf, walk, data, DepthFirst); + WMTreeWalk(leaf, walk, data); } } - - if (!DepthFirst) - (*walk)(aNode, data); } diff --git a/util/wmmenugen.c b/util/wmmenugen.c index f1ad830a..e0c9c9af 100644 --- a/util/wmmenugen.c +++ b/util/wmmenugen.c @@ -178,7 +178,7 @@ int main(int argc, char **argv) } WMSortTree(menu, menuSortFunc); - WMTreeWalk(menu, assemblePLMenuFunc, previousDepth, True); + WMTreeWalk(menu, assemblePLMenuFunc, previousDepth); i = WMGetArrayItemCount(plMenuNodes); if (i > 2) { /* more than one submenu unprocessed is almost certainly an error */ -- 2.39.5 From 9802b684aefeb7b092021e648582a5c371f63afe Mon Sep 17 00:00:00 2001 From: Stu Black Date: Fri, 31 Oct 2025 22:06:01 -0400 Subject: [PATCH 10/24] Rewrite WINGs/tree.c in Rust. This is a bit of a red herring, since WMTree is only used in wmmenugen, which I don't think I've ever directly used. See notes at the top of tree.rs for more musings on whether this should be in wutil-rs at all. --- WINGs/Makefile.am | 1 - WINGs/WINGs/WUtil.h | 8 +- WINGs/tree.c | 176 ------------------------------ util/Makefile.am | 2 +- util/wmmenugen.c | 14 +-- wutil-rs/Makefile.am | 1 + wutil-rs/src/lib.rs | 1 + wutil-rs/src/tree.rs | 252 +++++++++++++++++++++++++++++++++++++++++++ 8 files changed, 265 insertions(+), 190 deletions(-) delete mode 100644 WINGs/tree.c create mode 100644 wutil-rs/src/tree.rs diff --git a/WINGs/Makefile.am b/WINGs/Makefile.am index 0423a8be..4b2f4566 100644 --- a/WINGs/Makefile.am +++ b/WINGs/Makefile.am @@ -76,7 +76,6 @@ libWUtil_la_SOURCES = \ misc.c \ notification.c \ proplist.c \ - tree.c \ userdefaults.c \ userdefaults.h \ usleep.c \ diff --git a/WINGs/WINGs/WUtil.h b/WINGs/WINGs/WUtil.h index 6ca513a5..2797429c 100644 --- a/WINGs/WINGs/WUtil.h +++ b/WINGs/WINGs/WUtil.h @@ -615,14 +615,12 @@ void WMSetDataFormat(WMData *aData, unsigned format); unsigned WMGetDataFormat(WMData *aData); /* Storing data */ -/* ---[ WINGs/tree.c ]---------------------------------------------------- */ +/* ---[ wutil-rs/src/tree.rs ]---------------------------------------------------- */ /* Generic Tree and TreeNode */ WMTreeNode* WMCreateTreeNode(void *data); -WMTreeNode* WMCreateTreeNodeWithDestructor(void *data, WMFreeDataProc *destructor); - WMTreeNode* WMInsertItemInTree(WMTreeNode *parent, int index, void *item); #define WMAddItemToTree(parent, item) WMInsertItemInTree(parent, -1, item) @@ -636,11 +634,11 @@ void* WMGetDataForTreeNode(WMTreeNode *aNode); int WMGetTreeNodeDepth(WMTreeNode *aNode); /* Sort all tree recursively starting from the passed node */ -void WMSortTree(WMTreeNode *aNode, WMCompareDataProc *comparer); +void WMSortTree(WMTreeNode *aNode, int (*comparer)(const WMTreeNode *a, const WMTreeNode *b)); /* Returns the first node where node's data matches cdata by 'match' and node is * at most `limit' depths down from `aTree'. */ -WMTreeNode *WMFindInTreeWithDepthLimit(WMTreeNode * aTree, WMMatchDataProc * match, void *cdata, int limit); +WMTreeNode *WMFindInTreeWithDepthLimit(WMTreeNode * aTree, int (*match)(const WMTreeNode *item, const void *cdata), void *cdata, int limit); /* Walk every node of aNode with `walk' */ void WMTreeWalk(WMTreeNode *aNode, WMTreeWalkProc * walk, void *data); diff --git a/WINGs/tree.c b/WINGs/tree.c deleted file mode 100644 index 536e9e92..00000000 --- a/WINGs/tree.c +++ /dev/null @@ -1,176 +0,0 @@ - -#include - -#include "WUtil.h" - -typedef struct W_TreeNode { - void *data; - - /*unsigned int uflags:16; */ - - WMArray *leaves; - - int depth; - - WMFreeDataProc *destructor; -} W_TreeNode; - -static void destroyNode(void *data) -{ - WMTreeNode *aNode = (WMTreeNode *) data; - - if (aNode->destructor) { - (*aNode->destructor) (aNode->data); - } - if (aNode->leaves) { - WMFreeArray(aNode->leaves); - } - wfree(aNode); -} - -WMTreeNode *WMCreateTreeNode(void *data) -{ - return WMCreateTreeNodeWithDestructor(data, NULL); -} - -WMTreeNode *WMCreateTreeNodeWithDestructor(void *data, WMFreeDataProc * destructor) -{ - WMTreeNode *aNode; - - aNode = (WMTreeNode *) wmalloc(sizeof(W_TreeNode)); - aNode->destructor = destructor; - aNode->data = data; - aNode->depth = 0; - aNode->leaves = NULL; - /*aNode->leaves = WMCreateArrayWithDestructor(1, destroyNode); */ - - return aNode; -} - -WMTreeNode *WMInsertItemInTree(WMTreeNode * parent, int index, void *item) -{ - WMTreeNode *aNode; - - wassertrv(parent != NULL, NULL); - - aNode = WMCreateTreeNodeWithDestructor(item, parent->destructor); - aNode->depth = parent->depth + 1; - if (!parent->leaves) { - parent->leaves = WMCreateArrayWithDestructor(1, destroyNode); - } - if (index < 0) { - WMAddToArray(parent->leaves, aNode); - } else { - WMInsertInArray(parent->leaves, index, aNode); - } - - return aNode; -} - -static void updateNodeDepth(WMTreeNode * aNode, int depth) -{ - int i; - - aNode->depth = depth; - - if (aNode->leaves) { - for (i = 0; i < WMGetArrayItemCount(aNode->leaves); i++) { - updateNodeDepth(WMGetFromArray(aNode->leaves, i), depth + 1); - } - } -} - -WMTreeNode *WMInsertNodeInTree(WMTreeNode * parent, int index, WMTreeNode * aNode) -{ - wassertrv(parent != NULL, NULL); - wassertrv(aNode != NULL, NULL); - - updateNodeDepth(aNode, parent->depth + 1); - if (!parent->leaves) { - parent->leaves = WMCreateArrayWithDestructor(1, destroyNode); - } - if (index < 0) { - WMAddToArray(parent->leaves, aNode); - } else { - WMInsertInArray(parent->leaves, index, aNode); - } - - return aNode; -} - -void *WMGetDataForTreeNode(WMTreeNode * aNode) -{ - return aNode->data; -} - -int WMGetTreeNodeDepth(WMTreeNode * aNode) -{ - return aNode->depth; -} - -static void sortLeavesForNode(WMTreeNode * aNode, WMCompareDataProc * comparer) -{ - int i; - - if (!aNode->leaves) - return; - - WMSortArray(aNode->leaves, comparer); - for (i = 0; i < WMGetArrayItemCount(aNode->leaves); i++) { - sortLeavesForNode(WMGetFromArray(aNode->leaves, i), comparer); - } -} - -void WMSortTree(WMTreeNode * aNode, WMCompareDataProc * comparer) -{ - wassertr(aNode != NULL); - - sortLeavesForNode(aNode, comparer); -} - -static WMTreeNode *findNodeInTree(WMTreeNode * aNode, WMMatchDataProc * match, void *cdata, int limit) -{ - if (match == NULL && aNode->data == cdata) - return aNode; - else if (match && (*match) (aNode->data, cdata)) - return aNode; - - if (aNode->leaves && limit != 0) { - WMTreeNode *leaf; - int i; - - for (i = 0; i < WMGetArrayItemCount(aNode->leaves); i++) { - leaf = findNodeInTree(WMGetFromArray(aNode->leaves, i), - match, cdata, limit > 0 ? limit - 1 : limit); - if (leaf) - return leaf; - } - } - - return NULL; -} - -WMTreeNode *WMFindInTreeWithDepthLimit(WMTreeNode * aTree, WMMatchDataProc * match, void *cdata, int limit) -{ - wassertrv(aTree != NULL, NULL); - wassertrv(limit >= 0, NULL); - - return findNodeInTree(aTree, match, cdata, limit); -} - -void WMTreeWalk(WMTreeNode * aNode, WMTreeWalkProc * walk, void *data) -{ - int i; - WMTreeNode *leaf; - - wassertr(aNode != NULL); - - (*walk)(aNode, data); - - if (aNode->leaves) { - for (i = 0; i < WMGetArrayItemCount(aNode->leaves); i++) { - leaf = (WMTreeNode *)WMGetFromArray(aNode->leaves, i); - WMTreeWalk(leaf, walk, data); - } - } -} diff --git a/util/Makefile.am b/util/Makefile.am index dd026050..3fdd6d67 100644 --- a/util/Makefile.am +++ b/util/Makefile.am @@ -85,8 +85,8 @@ wmgenmenu_LDADD = \ wmgenmenu_SOURCES = wmgenmenu.c wmgenmenu.h wmmenugen_LDADD = \ - $(top_builddir)/WINGs/libWUtil.la \ $(top_builddir)/wutil-rs/target/debug/libwutil_rs.a \ + $(top_builddir)/WINGs/libWUtil.la \ @INTLIBS@ wmmenugen_SOURCES = wmmenugen.c wmmenugen.h wmmenugen_misc.c \ diff --git a/util/wmmenugen.c b/util/wmmenugen.c index e0c9c9af..713437c6 100644 --- a/util/wmmenugen.c +++ b/util/wmmenugen.c @@ -35,8 +35,8 @@ static void addWMMenuEntryCallback(WMMenuEntry *aEntry); static void assemblePLMenuFunc(WMTreeNode *aNode, void *data); static int dirParseFunc(const char *filename, const struct stat *st, int tflags, struct FTW *ftw); -static int menuSortFunc(const void *left, const void *right); -static int nodeFindSubMenuByNameFunc(const void *item, const void *cdata); +static int menuSortFunc(const WMTreeNode *left, const WMTreeNode *right); +static int nodeFindSubMenuByNameFunc(const WMTreeNode *tree, const void *cdata); static WMTreeNode *findPositionInMenu(const char *submenu); @@ -324,13 +324,13 @@ static void assemblePLMenuFunc(WMTreeNode *aNode, void *data) /* sort the menu tree; callback for WMSortTree() */ -static int menuSortFunc(const void *left, const void *right) +static int menuSortFunc(const WMTreeNode *left, const WMTreeNode *right) { WMMenuEntry *leftwm; WMMenuEntry *rightwm; - leftwm = (WMMenuEntry *)WMGetDataForTreeNode(*(WMTreeNode **)left); - rightwm = (WMMenuEntry *)WMGetDataForTreeNode(*(WMTreeNode **)right); + leftwm = (WMMenuEntry *)WMGetDataForTreeNode(left); + rightwm = (WMMenuEntry *)WMGetDataForTreeNode(right); /* submenus first */ if (!leftwm->CmdLine && rightwm->CmdLine) @@ -380,11 +380,11 @@ static WMTreeNode *findPositionInMenu(const char *submenu) /* find node where Name = cdata and node is a submenu */ -static int nodeFindSubMenuByNameFunc(const void *item, const void *cdata) +static int nodeFindSubMenuByNameFunc(const WMTreeNode *tree, const void *cdata) { WMMenuEntry *wm; - wm = (WMMenuEntry *)item; + wm = (WMMenuEntry *)WMGetDataForTreeNode(tree); if (wm->CmdLine) /* if it has a cmdline, it can't be a submenu */ return 0; diff --git a/wutil-rs/Makefile.am b/wutil-rs/Makefile.am index afbf427a..a8dab76f 100644 --- a/wutil-rs/Makefile.am +++ b/wutil-rs/Makefile.am @@ -11,6 +11,7 @@ RUST_SOURCES = \ src/memory.rs \ src/prop_list.rs \ src/string.rs + src/tree.rs RUST_EXTRA = \ Cargo.lock \ diff --git a/wutil-rs/src/lib.rs b/wutil-rs/src/lib.rs index 55adc6f7..c2315e11 100644 --- a/wutil-rs/src/lib.rs +++ b/wutil-rs/src/lib.rs @@ -6,3 +6,4 @@ pub mod hash_table; pub mod memory; pub mod prop_list; pub mod string; +pub mod tree; diff --git a/wutil-rs/src/tree.rs b/wutil-rs/src/tree.rs new file mode 100644 index 00000000..67e571fb --- /dev/null +++ b/wutil-rs/src/tree.rs @@ -0,0 +1,252 @@ +//! N-ary tree structure. +//! +//! ## Rust rewrite notes +//! +//! This is only used in one place (`util/wmmenugen.c`), and it should probably +//! be moved out of wutil-rs (if not deleted entirely) once we migrate that +//! utility. +//! +//! The FFI functions provided here assume that they will be used as they are in +//! `wmmenugen.c`. (For example, the C interface may allow for some callback +//! parameters to be null, but they aren't in practice, so the Rust layer +//! doesn't check for that.) +//! +//! The original C library had a larger surface area and tracked parent +//! pointers, but the Rust version only has the API used by wmmenugen. Case in +//! point: `WMTreeNode` originally had an optional destructor function pointer +//! for freeing its `data` field, but wmmenugen never actually deleted a tree +//! node, so the destructor was never called. This Rust rewrite doesn't track a +//! `data` destructor and doesn't even provide a way to delete tree nodes. +//! +//! If a generic tree really becomes necessary, [`Tree`] may be adapted, but a +//! different design is probably warranted. (At the very least, `Tree` should be +//! generic over the type of `data` so that we're not using `c_void`. It is also +//! probably better to index into an owned `Vec` or use an external +//! arena. `GhostCell` or one of its ilk may even make sense.) + +use std::ffi::c_void; + +pub struct Tree { + depth: u32, + children: Vec>, + data: *mut c_void, +} + +pub mod ffi { + use super::Tree; + + use std::{ + ffi::{c_int, c_void}, + ptr, + }; + + /// Creates the root of a new tree with the data `data`. + #[unsafe(no_mangle)] + pub unsafe extern "C" fn WMCreateTreeNode(data: *mut c_void) -> *mut Tree { + Box::leak(Box::new(Tree { + depth: 0, + children: Vec::new(), + data, + })) + } + + /// Creates a tree node as the `index`th child of `parent`, with the data + /// `item`. If `index` is negative, the node is added as the last child of + /// `parent`. + #[unsafe(no_mangle)] + pub unsafe extern "C" fn WMInsertItemInTree( + parent: *mut Tree, + index: c_int, + item: *mut c_void, + ) -> *mut Tree { + if parent.is_null() { + return ptr::null_mut(); + } + let parent = unsafe { &mut *parent }; + + let child = Tree { + depth: parent.depth + 1, + children: Vec::new(), + data: item, + }; + + if index < 0 { + parent.children.push(Box::new(child)); + parent.children.last_mut().unwrap().as_mut() as *mut _ + } else { + let index = index as usize; + parent.children.insert(index, Box::new(child)); + parent.children[index].as_mut() as *mut _ + } + } + + /// Inserts `tree` as the `index`th child of `parent`. If `index` is + /// negative, `tree` is added as the last child of `parent`. + /// + /// This eagerly updates the depth of the entire subtree rooted at `tree`, + /// so it has a O(N) cost rather than O(1). + #[unsafe(no_mangle)] + pub unsafe extern "C" fn WMInsertNodeInTree( + parent: *mut Tree, + index: c_int, + tree: *mut Tree, + ) -> *mut Tree { + if parent.is_null() { + return ptr::null_mut(); + } + let parent = unsafe { &mut *parent }; + + if tree.is_null() { + return ptr::null_mut(); + } + + let mut stack = vec![(unsafe { &mut *tree }, parent.depth + 1)]; + while let Some((tree, depth)) = stack.pop() { + tree.depth = depth; + for child in &mut tree.children { + stack.push((child, depth + 1)); + } + } + + if index < 0 { + parent.children.push(unsafe { Box::from_raw(tree) }); + tree + } else { + let index = index as usize; + parent + .children + .insert(index, unsafe { Box::from_raw(tree) }); + tree + } + } + + /// Returns the data field of `tree`. + #[unsafe(no_mangle)] + pub unsafe extern "C" fn WMGetDataForTreeNode(tree: *mut Tree) -> *mut c_void { + if tree.is_null() { + return ptr::null_mut(); + } + unsafe { (*tree).data } + } + + /// Returns the depth of `tree` (0 if `tree` is a root, 1 if it is an + /// immediate child of the root, etc.). This is an `O(1)` operation. + #[unsafe(no_mangle)] + pub unsafe extern "C" fn WMGetTreeNodeDepth(tree: *mut Tree) -> c_int { + if tree.is_null() { + return 0; + } + unsafe { (*tree).depth as c_int } + } + + /// Recursively sorts the children of each node of the subtree rooted at + /// `tree`, according to `comparer`. + #[unsafe(no_mangle)] + pub unsafe extern "C" fn WMSortTree( + tree: *mut Tree, + comparer: unsafe extern "C" fn(*const Tree, *const Tree) -> c_int, + ) { + use std::cmp::Ordering; + + if tree.is_null() { + return; + } + + let comparer = |a: &Box, b: &Box| { + let a = a.as_ref() as *const Tree as *const _; + let b = b.as_ref() as *const Tree as *const _; + match unsafe { comparer(a, b) }.signum() { + -1 => Ordering::Less, + 0 => Ordering::Equal, + 1 => Ordering::Greater, + _ => unreachable!(), + } + }; + + let mut stack = vec![unsafe { &mut *tree }]; + + while let Some(tree) = stack.pop() { + tree.children.sort_by(comparer); + stack.extend(tree.children.iter_mut().map(|c| c.as_mut())); + } + } + + /// Returns the first tree node in the subtree rooted at `tree` that matches + /// `match_p`, or null if none is found. If `match_p` is `None`, returns + /// the first node whose `data` field is equal to `cdata`. Search is + /// recursive, up to `limit` depth (which must be non-negative). + /// + /// ## Rust rewrite notes + /// + /// This was originally a DFS with a `depthFirst` parameter which actually + /// controlled whether tree traversal was pre- or post-order (and not + /// whether the search was depth-first or breadth-first, which the name + /// might seem to suggest). Since this was only ever called with a non-zero + /// `depthFirst`, the Rust version is hard-coded as a pre-order DFS. + #[unsafe(no_mangle)] + pub unsafe extern "C" fn WMFindInTreeWithDepthLimit( + tree: *mut Tree, + match_p: Option c_int>, + cdata: *mut c_void, + limit: c_int, + ) -> *mut Tree { + if tree.is_null() { + return ptr::null_mut(); + } + let safe_tree = unsafe { &mut *tree }; + + let match_p = |t: &Tree| -> bool { + if let Some(p) = match_p { + (unsafe { (p)(t, cdata) }) != 0 + } else { + t.data == cdata + } + }; + + let mut stack = vec![(safe_tree, 0)]; + + while let Some((tree, depth)) = stack.pop() { + if depth > limit { + continue; + } + + if match_p(tree) { + return tree as *mut Tree; + } + + for child in tree.children.iter_mut() { + stack.push((child.as_mut(), depth + 1)); + } + } + + ptr::null_mut() + } + + /// Traverses `tree` recursively, invoking `walk` on each tree node with + /// `cdata` passed in to provide a side channel. + #[unsafe(no_mangle)] + pub unsafe extern "C" fn WMTreeWalk( + tree: *mut Tree, + walk: unsafe extern "C" fn(*mut Tree, *mut c_void), + cdata: *mut c_void, + ) { + if tree.is_null() { + return; + } + let tree = unsafe { &mut *tree }; + + let walk_fn = |t: &mut Tree| unsafe { + walk(t as *mut Tree, cdata); + }; + + let mut stack = vec![tree]; + + while let Some(tree) = stack.pop() { + walk_fn(tree); + + for child in &mut tree.children { + stack.push(child); + } + } + } +} -- 2.39.5 From 46e540b1b18da2f0034106a67fc8c4fad37b40ee Mon Sep 17 00:00:00 2001 From: Stu Black Date: Mon, 27 Oct 2025 23:28:43 -0400 Subject: [PATCH 11/24] Prune the WMBag API in preparation for rewriting it in Rust. --- WINGs/WINGs/WUtil.h | 47 +--------- WINGs/bagtree.c | 214 -------------------------------------------- 2 files changed, 1 insertion(+), 260 deletions(-) diff --git a/WINGs/WINGs/WUtil.h b/WINGs/WINGs/WUtil.h index 2797429c..27e60723 100644 --- a/WINGs/WINGs/WUtil.h +++ b/WINGs/WINGs/WUtil.h @@ -494,58 +494,18 @@ void* WMArrayPrevious(WMArray *array, WMArrayIterator *iter); * Slow for storing small numbers of elements */ -#define WMCreateBag(size) WMCreateTreeBag() - -#define WMCreateBagWithDestructor(size, d) WMCreateTreeBagWithDestructor(d) - WMBag* WMCreateTreeBag(void); -WMBag* WMCreateTreeBagWithDestructor(WMFreeDataProc *destructor); - -int WMGetBagItemCount(WMBag *bag); - -void WMAppendBag(WMBag *bag, WMBag *other); - -void WMPutInBag(WMBag *bag, void *item); - -/* insert will increment the index of elements after it by 1 */ -void WMInsertInBag(WMBag *bag, int index, void *item); - -/* erase will remove the element from the bag, - * but will keep the index of the other elements unchanged */ -int WMEraseFromBag(WMBag *bag, int index); - -/* delete and remove will remove the elements and cause the elements - * after them to decrement their indexes by 1 */ -int WMDeleteFromBag(WMBag *bag, int index); - -int WMRemoveFromBag(WMBag *bag, void *item); - void* WMGetFromBag(WMBag *bag, int index); void* WMReplaceInBag(WMBag *bag, int index, void *item); #define WMSetInBag(bag, index, item) WMReplaceInBag(bag, index, item) -/* comparer must return: - * < 0 if a < b - * > 0 if a > b - * = 0 if a = b - */ -void WMSortBag(WMBag *bag, WMCompareDataProc *comparer); - void WMEmptyBag(WMBag *bag); void WMFreeBag(WMBag *bag); -void WMMapBag(WMBag *bag, void (*function)(void*, void*), void *data); - -int WMGetFirstInBag(WMBag *bag, void *item); - -int WMCountInBag(WMBag *bag, void *item); - -int WMFindInBag(WMBag *bag, WMMatchDataProc *match, void *cdata); - void* WMBagFirst(WMBag *bag, WMBagIterator *ptr); void* WMBagLast(WMBag *bag, WMBagIterator *ptr); @@ -557,13 +517,8 @@ void* WMBagPrevious(WMBag *bag, WMBagIterator *ptr); void* WMBagIteratorAtIndex(WMBag *bag, int index, WMBagIterator *ptr); -int WMBagIndexForIterator(WMBag *bag, WMBagIterator ptr); - -/* The following 2 macros assume that the bag doesn't change in the for loop */ -#define WM_ITERATE_BAG(bag, var, i) \ - for (var = WMBagFirst(bag, &(i)); (i) != NULL; \ - var = WMBagNext(bag, &(i))) +/* The following macro assumes that the bag doesn't change in the for loop */ #define WM_ETARETI_BAG(bag, var, i) \ for (var = WMBagLast(bag, &(i)); (i) != NULL; \ diff --git a/WINGs/bagtree.c b/WINGs/bagtree.c index 4b3062ca..52f7c6ed 100644 --- a/WINGs/bagtree.c +++ b/WINGs/bagtree.c @@ -21,7 +21,6 @@ typedef struct W_Bag { int count; - void (*destructor) (void *item); } W_Bag; #define IS_LEFT(node) (node == node->parent->left) @@ -349,11 +348,6 @@ void PrintTree(WMBag * bag) #endif WMBag *WMCreateTreeBag(void) -{ - return WMCreateTreeBagWithDestructor(NULL); -} - -WMBag *WMCreateTreeBagWithDestructor(WMFreeDataProc * destructor) { WMBag *bag; @@ -362,64 +356,10 @@ WMBag *WMCreateTreeBagWithDestructor(WMFreeDataProc * destructor) bag->nil->left = bag->nil->right = bag->nil->parent = bag->nil; bag->nil->index = WBNotFound; bag->root = bag->nil; - bag->destructor = destructor; return bag; } -int WMGetBagItemCount(WMBag * self) -{ - return self->count; -} - -void WMAppendBag(WMBag * self, WMBag * bag) -{ - WMBagIterator ptr; - void *data; - - for (data = WMBagFirst(bag, &ptr); data != NULL; data = WMBagNext(bag, &ptr)) { - WMPutInBag(self, data); - } -} - -void WMPutInBag(WMBag * self, void *item) -{ - W_Node *ptr; - - ptr = wmalloc(sizeof(W_Node)); - - ptr->data = item; - ptr->index = self->count; - ptr->left = self->nil; - ptr->right = self->nil; - ptr->parent = self->nil; - - rbTreeInsert(self, ptr); - - self->count++; -} - -void WMInsertInBag(WMBag * self, int index, void *item) -{ - W_Node *ptr; - - ptr = wmalloc(sizeof(W_Node)); - - ptr->data = item; - ptr->index = index; - ptr->left = self->nil; - ptr->right = self->nil; - ptr->parent = self->nil; - - rbTreeInsert(self, ptr); - - while ((ptr = treeSuccessor(ptr, self->nil)) != self->nil) { - ptr->index++; - } - - self->count++; -} - static int treeDeleteNode(WMBag * self, W_Node *ptr) { if (ptr != self->nil) { @@ -434,47 +374,12 @@ static int treeDeleteNode(WMBag * self, W_Node *ptr) } ptr = rbTreeDelete(self, ptr); - if (self->destructor) - self->destructor(ptr->data); wfree(ptr); return 1; } return 0; } -int WMRemoveFromBag(WMBag * self, void *item) -{ - W_Node *ptr = treeFind(self->root, self->nil, item); - return treeDeleteNode(self, ptr); -} - -int WMEraseFromBag(WMBag * self, int index) -{ - W_Node *ptr = treeSearch(self->root, self->nil, index); - - if (ptr != self->nil) { - - self->count--; - - ptr = rbTreeDelete(self, ptr); - if (self->destructor) - self->destructor(ptr->data); - wfree(ptr); - - wassertrv(self->count == 0 || self->root->index >= 0, 1); - - return 1; - } else { - return 0; - } -} - -int WMDeleteFromBag(WMBag * self, int index) -{ - W_Node *ptr = treeSearch(self->root, self->nil, index); - return treeDeleteNode(self, ptr); -} - void *WMGetFromBag(WMBag * self, int index) { W_Node *node; @@ -486,41 +391,6 @@ void *WMGetFromBag(WMBag * self, int index) return NULL; } -int WMGetFirstInBag(WMBag * self, void *item) -{ - W_Node *node; - - node = treeFind(self->root, self->nil, item); - if (node != self->nil) - return node->index; - else - return WBNotFound; -} - -static int treeCount(W_Node * root, W_Node * nil, void *item) -{ - int count = 0; - - if (root == nil) - return 0; - - if (root->data == item) - count++; - - if (root->left != nil) - count += treeCount(root->left, nil, item); - - if (root->right != nil) - count += treeCount(root->right, nil, item); - - return count; -} - -int WMCountInBag(WMBag * self, void *item) -{ - return treeCount(self->root, self->nil, item); -} - void *WMReplaceInBag(WMBag * self, int index, void *item) { W_Node *ptr = treeSearch(self->root, self->nil, index); @@ -529,8 +399,6 @@ void *WMReplaceInBag(WMBag * self, int index, void *item) if (item == NULL) { self->count--; ptr = rbTreeDelete(self, ptr); - if (self->destructor) - self->destructor(ptr->data); wfree(ptr); } else if (ptr != self->nil) { old = ptr->data; @@ -554,37 +422,6 @@ void *WMReplaceInBag(WMBag * self, int index, void *item) return old; } -void WMSortBag(WMBag * self, WMCompareDataProc * comparer) -{ - void **items; - W_Node *tmp; - int i; - - if (self->count == 0) - return; - - items = wmalloc(sizeof(void *) * self->count); - i = 0; - - tmp = treeMinimum(self->root, self->nil); - while (tmp != self->nil) { - items[i++] = tmp->data; - tmp = treeSuccessor(tmp, self->nil); - } - - qsort(&items[0], self->count, sizeof(void *), comparer); - - i = 0; - tmp = treeMinimum(self->root, self->nil); - while (tmp != self->nil) { - tmp->index = i; - tmp->data = items[i++]; - tmp = treeSuccessor(tmp, self->nil); - } - - wfree(items); -} - static void deleteTree(WMBag * self, W_Node * node) { if (node == self->nil) @@ -592,9 +429,6 @@ static void deleteTree(WMBag * self, W_Node * node) deleteTree(self, node->left); - if (self->destructor) - self->destructor(node->data); - deleteTree(self, node->right); wfree(node); @@ -614,46 +448,6 @@ void WMFreeBag(WMBag * self) wfree(self); } -static void mapTree(W_Bag * tree, W_Node * node, void (*function) (void *, void *), void *data) -{ - if (node == tree->nil) - return; - - mapTree(tree, node->left, function, data); - - (*function) (node->data, data); - - mapTree(tree, node->right, function, data); -} - -void WMMapBag(WMBag * self, void (*function) (void *, void *), void *data) -{ - mapTree(self, self->root, function, data); -} - -static int findInTree(W_Bag * tree, W_Node * node, WMMatchDataProc * function, void *cdata) -{ - int index; - - if (node == tree->nil) - return WBNotFound; - - index = findInTree(tree, node->left, function, cdata); - if (index != WBNotFound) - return index; - - if ((*function) (node->data, cdata)) { - return node->index; - } - - return findInTree(tree, node->right, function, cdata); -} - -int WMFindInBag(WMBag * self, WMMatchDataProc * match, void *cdata) -{ - return findInTree(self, self->root, match, cdata); -} - void *WMBagFirst(WMBag * self, WMBagIterator * ptr) { W_Node *node; @@ -735,11 +529,3 @@ void *WMBagIteratorAtIndex(WMBag * self, int index, WMBagIterator * ptr) return node->data; } } - -int WMBagIndexForIterator(WMBag * bag, WMBagIterator ptr) -{ - /* Parameter not used, but tell the compiler that it is ok */ - (void) bag; - - return ((W_Node *) ptr)->index; -} -- 2.39.5 From 89183f3bcb9ddbebea430d075389bf5bb1bb1076 Mon Sep 17 00:00:00 2001 From: Stu Black Date: Tue, 28 Oct 2025 21:13:31 -0400 Subject: [PATCH 12/24] Rewrite WMBag in Rust. We should eventually get rid of this entirely, in favor of something along the lines of a sorted Vec that is fully typed (so the payload isn't just a void*). --- WINGs/Makefile.am | 1 - WINGs/WINGs/WUtil.h | 10 +- WINGs/bagtree.c | 531 ------------------------------------------- src/shutdown.c | 11 +- wutil-rs/Makefile.am | 1 + wutil-rs/src/bag.rs | 390 +++++++++++++++++++++++++++++++ wutil-rs/src/lib.rs | 1 + 7 files changed, 402 insertions(+), 543 deletions(-) delete mode 100644 WINGs/bagtree.c create mode 100644 wutil-rs/src/bag.rs diff --git a/WINGs/Makefile.am b/WINGs/Makefile.am index 4b2f4566..74b3ffea 100644 --- a/WINGs/Makefile.am +++ b/WINGs/Makefile.am @@ -65,7 +65,6 @@ libWINGs_la_SOURCES = \ wwindow.c libWUtil_la_SOURCES = \ - bagtree.c \ error.c \ error.h \ findfile.c \ diff --git a/WINGs/WINGs/WUtil.h b/WINGs/WINGs/WUtil.h index 27e60723..060c1adb 100644 --- a/WINGs/WINGs/WUtil.h +++ b/WINGs/WINGs/WUtil.h @@ -173,7 +173,7 @@ typedef struct { typedef int WMArrayIterator; -typedef void *WMBagIterator; +typedef int WMBagIterator; typedef void WMNotificationObserverAction(void *observerData, @@ -479,7 +479,7 @@ void* WMArrayPrevious(WMArray *array, WMArrayIterator *iter); for (var = WMArrayLast(array, &(i)); (i) != WANotFound; \ var = WMArrayPrevious(array, &(i))) -/* ---[ WINGs/bagtree.c ]------------------------------------------------- */ +/* ---[ wutil-rs/src/bag.rs ]------------------------------------------------- */ /* * Tree bags use a red-black tree for storage. @@ -498,9 +498,7 @@ WMBag* WMCreateTreeBag(void); void* WMGetFromBag(WMBag *bag, int index); -void* WMReplaceInBag(WMBag *bag, int index, void *item); - -#define WMSetInBag(bag, index, item) WMReplaceInBag(bag, index, item) +void WMSetInBag(WMBag *bag, int index, void *item); void WMEmptyBag(WMBag *bag); @@ -521,7 +519,7 @@ void* WMBagIteratorAtIndex(WMBag *bag, int index, WMBagIterator *ptr); /* The following macro assumes that the bag doesn't change in the for loop */ #define WM_ETARETI_BAG(bag, var, i) \ - for (var = WMBagLast(bag, &(i)); (i) != NULL; \ + for (var = WMBagLast(bag, &(i)); (i) >= 0; \ var = WMBagPrevious(bag, &(i))) diff --git a/WINGs/bagtree.c b/WINGs/bagtree.c deleted file mode 100644 index 52f7c6ed..00000000 --- a/WINGs/bagtree.c +++ /dev/null @@ -1,531 +0,0 @@ - -#include -#include - -#include "WUtil.h" - -typedef struct W_Node { - struct W_Node *parent; - struct W_Node *left; - struct W_Node *right; - int color; - - void *data; - int index; -} W_Node; - -typedef struct W_Bag { - W_Node *root; - - W_Node *nil; /* sentinel */ - - int count; - -} W_Bag; - -#define IS_LEFT(node) (node == node->parent->left) -#define IS_RIGHT(node) (node == node->parent->right) - -static void leftRotate(W_Bag * tree, W_Node * node) -{ - W_Node *node2; - - node2 = node->right; - node->right = node2->left; - - node2->left->parent = node; - - node2->parent = node->parent; - - if (node->parent == tree->nil) { - tree->root = node2; - } else { - if (IS_LEFT(node)) { - node->parent->left = node2; - } else { - node->parent->right = node2; - } - } - node2->left = node; - node->parent = node2; -} - -static void rightRotate(W_Bag * tree, W_Node * node) -{ - W_Node *node2; - - node2 = node->left; - node->left = node2->right; - - node2->right->parent = node; - - node2->parent = node->parent; - - if (node->parent == tree->nil) { - tree->root = node2; - } else { - if (IS_LEFT(node)) { - node->parent->left = node2; - } else { - node->parent->right = node2; - } - } - node2->right = node; - node->parent = node2; -} - -static void treeInsert(W_Bag * tree, W_Node * node) -{ - W_Node *y = tree->nil; - W_Node *x = tree->root; - - while (x != tree->nil) { - y = x; - if (node->index <= x->index) - x = x->left; - else - x = x->right; - } - node->parent = y; - if (y == tree->nil) - tree->root = node; - else if (node->index <= y->index) - y->left = node; - else - y->right = node; -} - -static void rbTreeInsert(W_Bag * tree, W_Node * node) -{ - W_Node *y; - - treeInsert(tree, node); - - node->color = 'R'; - - while (node != tree->root && node->parent->color == 'R') { - if (IS_LEFT(node->parent)) { - y = node->parent->parent->right; - - if (y->color == 'R') { - - node->parent->color = 'B'; - y->color = 'B'; - node->parent->parent->color = 'R'; - node = node->parent->parent; - - } else { - if (IS_RIGHT(node)) { - node = node->parent; - leftRotate(tree, node); - } - node->parent->color = 'B'; - node->parent->parent->color = 'R'; - rightRotate(tree, node->parent->parent); - } - } else { - y = node->parent->parent->left; - - if (y->color == 'R') { - - node->parent->color = 'B'; - y->color = 'B'; - node->parent->parent->color = 'R'; - node = node->parent->parent; - - } else { - if (IS_LEFT(node)) { - node = node->parent; - rightRotate(tree, node); - } - node->parent->color = 'B'; - node->parent->parent->color = 'R'; - leftRotate(tree, node->parent->parent); - } - } - } - tree->root->color = 'B'; -} - -static void rbDeleteFixup(W_Bag * tree, W_Node * node) -{ - W_Node *w; - - while (node != tree->root && node->color == 'B') { - if (IS_LEFT(node)) { - w = node->parent->right; - if (w->color == 'R') { - w->color = 'B'; - node->parent->color = 'R'; - leftRotate(tree, node->parent); - w = node->parent->right; - } - if (w->left->color == 'B' && w->right->color == 'B') { - w->color = 'R'; - node = node->parent; - } else { - if (w->right->color == 'B') { - w->left->color = 'B'; - w->color = 'R'; - rightRotate(tree, w); - w = node->parent->right; - } - w->color = node->parent->color; - node->parent->color = 'B'; - w->right->color = 'B'; - leftRotate(tree, node->parent); - node = tree->root; - } - } else { - w = node->parent->left; - if (w->color == 'R') { - w->color = 'B'; - node->parent->color = 'R'; - rightRotate(tree, node->parent); - w = node->parent->left; - } - if (w->left->color == 'B' && w->right->color == 'B') { - w->color = 'R'; - node = node->parent; - } else { - if (w->left->color == 'B') { - w->right->color = 'B'; - w->color = 'R'; - leftRotate(tree, w); - w = node->parent->left; - } - w->color = node->parent->color; - node->parent->color = 'B'; - w->left->color = 'B'; - rightRotate(tree, node->parent); - node = tree->root; - } - } - } - node->color = 'B'; - -} - -static W_Node *treeMinimum(W_Node * node, W_Node * nil) -{ - while (node->left != nil) - node = node->left; - return node; -} - -static W_Node *treeMaximum(W_Node * node, W_Node * nil) -{ - while (node->right != nil) - node = node->right; - return node; -} - -static W_Node *treeSuccessor(W_Node * node, W_Node * nil) -{ - W_Node *y; - - if (node->right != nil) { - return treeMinimum(node->right, nil); - } - y = node->parent; - while (y != nil && node == y->right) { - node = y; - y = y->parent; - } - return y; -} - -static W_Node *treePredecessor(W_Node * node, W_Node * nil) -{ - W_Node *y; - - if (node->left != nil) { - return treeMaximum(node->left, nil); - } - y = node->parent; - while (y != nil && node == y->left) { - node = y; - y = y->parent; - } - return y; -} - -static W_Node *rbTreeDelete(W_Bag * tree, W_Node * node) -{ - W_Node *nil = tree->nil; - W_Node *x, *y; - - if (node->left == nil || node->right == nil) { - y = node; - } else { - y = treeSuccessor(node, nil); - } - - if (y->left != nil) { - x = y->left; - } else { - x = y->right; - } - - x->parent = y->parent; - - if (y->parent == nil) { - tree->root = x; - } else { - if (IS_LEFT(y)) { - y->parent->left = x; - } else { - y->parent->right = x; - } - } - if (y != node) { - node->index = y->index; - node->data = y->data; - } - if (y->color == 'B') { - rbDeleteFixup(tree, x); - } - - return y; -} - -static W_Node *treeSearch(W_Node * root, W_Node * nil, int index) -{ - if (root == nil || root->index == index) { - return root; - } - - if (index < root->index) { - return treeSearch(root->left, nil, index); - } else { - return treeSearch(root->right, nil, index); - } -} - -static W_Node *treeFind(W_Node * root, W_Node * nil, void *data) -{ - W_Node *tmp; - - if (root == nil || root->data == data) - return root; - - tmp = treeFind(root->left, nil, data); - if (tmp != nil) - return tmp; - - tmp = treeFind(root->right, nil, data); - - return tmp; -} - -#if 0 -static char buf[512]; - -static void printNodes(W_Node * node, W_Node * nil, int depth) -{ - if (node == nil) { - return; - } - - printNodes(node->left, nil, depth + 1); - - memset(buf, ' ', depth * 2); - buf[depth * 2] = 0; - if (IS_LEFT(node)) - printf("%s/(%2i\n", buf, node->index); - else - printf("%s\\(%2i\n", buf, node->index); - - printNodes(node->right, nil, depth + 1); -} - -void PrintTree(WMBag * bag) -{ - W_TreeBag *tree = (W_TreeBag *) bag->data; - - printNodes(tree->root, tree->nil, 0); -} -#endif - -WMBag *WMCreateTreeBag(void) -{ - WMBag *bag; - - bag = wmalloc(sizeof(WMBag)); - bag->nil = wmalloc(sizeof(W_Node)); - bag->nil->left = bag->nil->right = bag->nil->parent = bag->nil; - bag->nil->index = WBNotFound; - bag->root = bag->nil; - - return bag; -} - -static int treeDeleteNode(WMBag * self, W_Node *ptr) -{ - if (ptr != self->nil) { - W_Node *tmp; - - self->count--; - - tmp = treeSuccessor(ptr, self->nil); - while (tmp != self->nil) { - tmp->index--; - tmp = treeSuccessor(tmp, self->nil); - } - - ptr = rbTreeDelete(self, ptr); - wfree(ptr); - return 1; - } - return 0; -} - -void *WMGetFromBag(WMBag * self, int index) -{ - W_Node *node; - - node = treeSearch(self->root, self->nil, index); - if (node != self->nil) - return node->data; - else - return NULL; -} - -void *WMReplaceInBag(WMBag * self, int index, void *item) -{ - W_Node *ptr = treeSearch(self->root, self->nil, index); - void *old = NULL; - - if (item == NULL) { - self->count--; - ptr = rbTreeDelete(self, ptr); - wfree(ptr); - } else if (ptr != self->nil) { - old = ptr->data; - ptr->data = item; - } else { - W_Node *ptr; - - ptr = wmalloc(sizeof(W_Node)); - - ptr->data = item; - ptr->index = index; - ptr->left = self->nil; - ptr->right = self->nil; - ptr->parent = self->nil; - - rbTreeInsert(self, ptr); - - self->count++; - } - - return old; -} - -static void deleteTree(WMBag * self, W_Node * node) -{ - if (node == self->nil) - return; - - deleteTree(self, node->left); - - deleteTree(self, node->right); - - wfree(node); -} - -void WMEmptyBag(WMBag * self) -{ - deleteTree(self, self->root); - self->root = self->nil; - self->count = 0; -} - -void WMFreeBag(WMBag * self) -{ - WMEmptyBag(self); - wfree(self->nil); - wfree(self); -} - -void *WMBagFirst(WMBag * self, WMBagIterator * ptr) -{ - W_Node *node; - - node = treeMinimum(self->root, self->nil); - - if (node == self->nil) { - *ptr = NULL; - return NULL; - } else { - *ptr = node; - return node->data; - } -} - -void *WMBagLast(WMBag * self, WMBagIterator * ptr) -{ - - W_Node *node; - - node = treeMaximum(self->root, self->nil); - - if (node == self->nil) { - *ptr = NULL; - return NULL; - } else { - *ptr = node; - return node->data; - } -} - -void *WMBagNext(WMBag * self, WMBagIterator * ptr) -{ - W_Node *node; - - if (*ptr == NULL) - return NULL; - - node = treeSuccessor(*ptr, self->nil); - - if (node == self->nil) { - *ptr = NULL; - return NULL; - } else { - *ptr = node; - return node->data; - } -} - -void *WMBagPrevious(WMBag * self, WMBagIterator * ptr) -{ - W_Node *node; - - if (*ptr == NULL) - return NULL; - - node = treePredecessor(*ptr, self->nil); - - if (node == self->nil) { - *ptr = NULL; - return NULL; - } else { - *ptr = node; - return node->data; - } -} - -void *WMBagIteratorAtIndex(WMBag * self, int index, WMBagIterator * ptr) -{ - W_Node *node; - - node = treeSearch(self->root, self->nil, index); - - if (node == self->nil) { - *ptr = NULL; - return NULL; - } else { - *ptr = node; - return node->data; - } -} diff --git a/src/shutdown.c b/src/shutdown.c index 300a2692..89ac34e9 100644 --- a/src/shutdown.c +++ b/src/shutdown.c @@ -109,16 +109,16 @@ void Shutdown(WShutdownMode mode) } } -static void restoreWindows(WMBag * bag, WMBagIterator iter) +static void restoreWindows(WMBag * bag, WMBagIterator *iter) { WCoreWindow *next; WCoreWindow *core; WWindow *wwin; - if (iter == NULL) { - core = WMBagFirst(bag, &iter); + if (*iter < 0) { + core = WMBagFirst(bag, iter); } else { - core = WMBagNext(bag, &iter); + core = WMBagNext(bag, iter); } if (core == NULL) @@ -168,7 +168,8 @@ void RestoreDesktop(WScreen * scr) wDestroyInspectorPanels(); /* reparent windows back to the root window, keeping the stacking order */ - restoreWindows(scr->stacking_list, NULL); + WMBagIterator iter = -1; + restoreWindows(scr->stacking_list, &iter); XUngrabServer(dpy); XSetInputFocus(dpy, PointerRoot, RevertToParent, CurrentTime); diff --git a/wutil-rs/Makefile.am b/wutil-rs/Makefile.am index a8dab76f..e2ee2d06 100644 --- a/wutil-rs/Makefile.am +++ b/wutil-rs/Makefile.am @@ -2,6 +2,7 @@ AUTOMAKE_OPTIONS = RUST_SOURCES = \ src/array.rs \ + src/bag.rs \ src/data.rs \ src/defines.c \ src/defines.rs \ diff --git a/wutil-rs/src/bag.rs b/wutil-rs/src/bag.rs new file mode 100644 index 00000000..afe71ea7 --- /dev/null +++ b/wutil-rs/src/bag.rs @@ -0,0 +1,390 @@ +//! Simple sorted set. +//! +//! ## Rust rewrite notes +//! +//! This was originally a full-blown red-black tree, but it was really only used +//! to keep a sorted list. It has been rewritten as a sorted `Vec`. We +//! technically no longer have O(log(n)) insertion time. Given set sizes and +//! actual execution time, that shouldn't matter. +//! +//! Prefer a proper Rust collection over this for new code. + +use std::{ + ffi::{c_int, c_void}, + ptr::NonNull, +}; + +#[derive(Default)] +pub struct Bag { + inner: Vec<(c_int, NonNull)>, +} + +/// Sentinel iterator value. In C, just check for negative values. +pub const NOT_FOUND: c_int = -1; + +impl Bag { + /// Returns `Ok(index)` if an item keyed by `key` is in `self`, else + /// `Err(index)` if an item keyed by `key` could be inserted at `index`. + fn search(&self, key: c_int) -> Result { + // self.inner.binary_search_by(|x| key.cmp(&x.0)) + self.inner.binary_search_by_key(&key, |(key, _)| *key) + } + + /// Returns the index of `self.inner` where `key` is found. + fn find_index(&self, key: c_int) -> Option { + self.search(key).ok() + } + + /// Returns the value associated with `key`. + fn find_value(&self, key: c_int) -> Option> { + let i = self.find_index(key)?; + self.inner.get(i).copied().map(|(_, value)| value) + } + + /// Removes any value associated with `key`. + fn remove(&mut self, key: c_int) { + let Some(i) = self.find_index(key) else { + return; + }; + self.inner.remove(i); + } + + /// Sets a value associated with `key` to `value`. Clobbers any extant value. + fn set(&mut self, key: c_int, value: NonNull) { + match self.search(key) { + Ok(i) => self.inner[i] = (key, value), + Err(i) => self.inner.insert(i, (key, value)), + } + } +} + +pub mod ffi { + use super::{Bag, NOT_FOUND}; + + use std::{ + ffi::{c_int, c_void}, + ptr::{self, NonNull}, + }; + + /// Basic constructor. Free with [`WMFreeBag`]. + #[unsafe(no_mangle)] + pub unsafe extern "C" fn WMCreateTreeBag() -> *mut Bag { + Box::leak(Box::new(Bag::default())) + } + + /// Retrieves the value associated with `key`, or null. + #[unsafe(no_mangle)] + pub unsafe extern "C" fn WMGetFromBag(bag: *mut Bag, key: c_int) -> *mut c_void { + if bag.is_null() { + return ptr::null_mut(); + } + unsafe { + (*bag) + .find_value(key) + .map(|p| p.as_ptr()) + .unwrap_or(ptr::null_mut()) + } + } + + /// Sets the value associated with `key` to `item`. If `item` is null, + /// removes any extant value. + #[unsafe(no_mangle)] + pub unsafe extern "C" fn WMSetInBag(bag: *mut Bag, key: c_int, item: *mut c_void) { + if bag.is_null() { + return; + } + let bag = unsafe { &mut *bag }; + + if let Some(item) = NonNull::new(item) { + bag.set(key, item); + } else { + bag.remove(key); + return; + } + } + + /// Clears the contents of `bag`. + #[unsafe(no_mangle)] + pub unsafe extern "C" fn WMEmptyBag(bag: *mut Bag) { + if bag.is_null() { + return; + } + unsafe { + (*bag).inner.clear(); + } + } + + /// Deletes `bag`. + #[unsafe(no_mangle)] + pub unsafe extern "C" fn WMFreeBag(bag: *mut Bag) { + if bag.is_null() { + return; + } + unsafe { + let _ = Box::from_raw(bag); + } + } + + /// Initializes `ptr` to the first element of `bag` and returns that + /// element. Sets `ptr` to [`NOT_FOUND`] and returns null if `bag` is empty. + #[unsafe(no_mangle)] + pub unsafe extern "C" fn WMBagFirst(bag: *mut Bag, ptr: *mut c_int) -> *mut c_void { + if ptr.is_null() { + return ptr::null_mut(); + } + let ptr = unsafe { &mut *ptr }; + + if bag.is_null() { + *ptr = NOT_FOUND; + return ptr::null_mut(); + } + let bag = unsafe { &mut *bag }; + + if let Some((_, value)) = bag.inner.first() { + *ptr = 0; + value.as_ptr() + } else { + *ptr = NOT_FOUND; + ptr::null_mut() + } + } + + /// Initializes `ptr` to the last element of `bag` and returns that + /// element. Sets `ptr` to [`NOT_FOUND`] and returns null if `bag` is empty. + #[unsafe(no_mangle)] + pub unsafe extern "C" fn WMBagLast(bag: *mut Bag, ptr: *mut c_int) -> *mut c_void { + if ptr.is_null() { + return ptr::null_mut(); + } + let ptr = unsafe { &mut *ptr }; + + if bag.is_null() { + *ptr = NOT_FOUND; + return ptr::null_mut(); + } + let bag = unsafe { &mut *bag }; + + if let Some((_, value)) = bag.inner.last() { + *ptr = (bag.inner.len() - 1) as c_int; + value.as_ptr() + } else { + *ptr = NOT_FOUND; + ptr::null_mut() + } + } + + /// Advances `ptr` to the next element of `bag` and returns it. Sets `ptr` + /// to [`NOT_FOUND`] and returns null if `ptr` is already at the end of + /// `bag`. + #[unsafe(no_mangle)] + pub unsafe extern "C" fn WMBagNext(bag: *mut Bag, ptr: *mut c_int) -> *mut c_void { + if ptr.is_null() { + return ptr::null_mut(); + } + let ptr = unsafe { &mut *ptr }; + + if bag.is_null() { + *ptr = NOT_FOUND; + return ptr::null_mut(); + } + let bag = unsafe { &mut *bag }; + + if *ptr < 0 { + return ptr::null_mut(); + } + + *ptr += 1; + if let Some((_, value)) = bag.inner.get(*ptr as usize) { + value.as_ptr() + } else { + *ptr = NOT_FOUND; + ptr::null_mut() + } + } + + /// Decrements `ptr` to the previous element of `bag` and returns it. Sets `ptr` + /// to [`NOT_FOUND`] and returns null if `ptr` is already at the beginning of + /// `bag`. + #[unsafe(no_mangle)] + pub unsafe extern "C" fn WMBagPrevious(bag: *mut Bag, ptr: *mut c_int) -> *mut c_void { + if ptr.is_null() { + return ptr::null_mut(); + } + let ptr = unsafe { &mut *ptr }; + + if bag.is_null() { + *ptr = NOT_FOUND; + return ptr::null_mut(); + } + let bag = unsafe { &mut *bag }; + + if *ptr <= 0 { + *ptr = NOT_FOUND; + return ptr::null_mut(); + } + *ptr -= 1; + if let Some((_, value)) = bag.inner.get(*ptr as usize) { + value.as_ptr() + } else { + *ptr = NOT_FOUND; + ptr::null_mut() + } + } + + /// Sets `ptr` to the element of `bag` with `key` and returns the associated + /// value. Sets `ptr` to [`NOT_FOUND`] and returns null if there is no such + /// element. + #[unsafe(no_mangle)] + pub unsafe extern "C" fn WMBagIteratorAtIndex( + bag: *mut Bag, + key: c_int, + ptr: *mut c_int, + ) -> *mut c_void { + if ptr.is_null() { + return ptr::null_mut(); + } + let ptr = unsafe { &mut *ptr }; + + if bag.is_null() { + *ptr = NOT_FOUND; + return ptr::null_mut(); + } + let bag = unsafe { &mut *bag }; + + if key < 0 { + *ptr = NOT_FOUND; + return ptr::null_mut(); + } + + if let Some(index) = bag.find_index(key) { + *ptr = index as c_int; + bag.inner[index].1.as_ptr() + } else { + *ptr = NOT_FOUND; + ptr::null_mut() + } + } +} + +#[cfg(test)] +mod test { + use super::{ffi, Bag, NOT_FOUND}; + + use std::{ + ffi::{c_int, c_void}, + ptr::NonNull, + }; + + fn void_of(t: &mut T) -> NonNull { + NonNull::new(t as *mut _ as *mut c_void).unwrap() + } + + #[test] + fn insert_out_of_order() { + let mut b = Bag::default(); + let mut x = 3u16; + let mut y = 4u16; + let mut z = 5u16; + b.set(7, void_of(&mut z)); + b.set(3, void_of(&mut y)); + b.set(5, void_of(&mut x)); + + assert_eq!( + b.inner, + &[ + (3, void_of(&mut y)), + (5, void_of(&mut x)), + (7, void_of(&mut z)) + ] + ); + } + + #[test] + fn iterate_forward() { + let mut b = Bag::default(); + let mut x = 3u16; + let mut y = 4u16; + let mut z = 5u16; + b.set(7, void_of(&mut z)); + b.set(3, void_of(&mut y)); + b.set(5, void_of(&mut x)); + + let mut iterator: c_int = NOT_FOUND; + unsafe { + assert_eq!( + ffi::WMBagFirst(&mut b, &mut iterator), + void_of(&mut y).as_ptr() + ); + assert_eq!(iterator, 0); + assert_eq!( + ffi::WMBagNext(&mut b, &mut iterator), + void_of(&mut x).as_ptr() + ); + assert_eq!(iterator, 1); + assert_eq!( + ffi::WMBagNext(&mut b, &mut iterator), + void_of(&mut z).as_ptr() + ); + assert_eq!(iterator, 2); + assert!(ffi::WMBagNext(&mut b, &mut iterator).is_null()); + assert_eq!(iterator, NOT_FOUND); + } + } + + #[test] + fn iterate_backward() { + let mut b = Bag::default(); + let mut x = 3u16; + let mut y = 4u16; + let mut z = 5u16; + b.set(7, void_of(&mut z)); + b.set(3, void_of(&mut y)); + b.set(5, void_of(&mut x)); + + let mut iterator: c_int = NOT_FOUND; + unsafe { + assert_eq!( + ffi::WMBagLast(&mut b, &mut iterator), + void_of(&mut z).as_ptr() + ); + assert_eq!(iterator, 2); + assert_eq!( + ffi::WMBagPrevious(&mut b, &mut iterator), + void_of(&mut x).as_ptr() + ); + assert_eq!(iterator, 1); + assert_eq!( + ffi::WMBagPrevious(&mut b, &mut iterator), + void_of(&mut y).as_ptr() + ); + assert_eq!(iterator, 0); + assert!(ffi::WMBagPrevious(&mut b, &mut iterator).is_null()); + assert_eq!(iterator, NOT_FOUND); + } + } + + #[test] + fn find_iterate() { + let mut b = Bag::default(); + let mut x = 3u16; + let mut y = 4u16; + let mut z = 5u16; + b.set(7, void_of(&mut z)); + b.set(3, void_of(&mut y)); + b.set(5, void_of(&mut x)); + + let mut iterator: c_int = NOT_FOUND; + unsafe { + assert_eq!( + ffi::WMBagIteratorAtIndex(&mut b, 5, &mut iterator), + void_of(&mut x).as_ptr() + ); + assert_eq!(iterator, 1); + assert_eq!( + ffi::WMBagNext(&mut b, &mut iterator), + void_of(&mut z).as_ptr() + ); + assert_eq!(iterator, 2); + } + } +} diff --git a/wutil-rs/src/lib.rs b/wutil-rs/src/lib.rs index c2315e11..ab94d66d 100644 --- a/wutil-rs/src/lib.rs +++ b/wutil-rs/src/lib.rs @@ -1,4 +1,5 @@ pub mod array; +pub mod bag; pub mod data; pub mod defines; pub mod find_file; -- 2.39.5 From 8270124869575420e96cf4f6f03c43343f1feb88 Mon Sep 17 00:00:00 2001 From: Stu Black Date: Wed, 12 Nov 2025 17:03:23 -0500 Subject: [PATCH 13/24] Nix dead code that was left commented out. --- wutil-rs/src/bag.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/wutil-rs/src/bag.rs b/wutil-rs/src/bag.rs index afe71ea7..5115c91f 100644 --- a/wutil-rs/src/bag.rs +++ b/wutil-rs/src/bag.rs @@ -26,7 +26,6 @@ impl Bag { /// Returns `Ok(index)` if an item keyed by `key` is in `self`, else /// `Err(index)` if an item keyed by `key` could be inserted at `index`. fn search(&self, key: c_int) -> Result { - // self.inner.binary_search_by(|x| key.cmp(&x.0)) self.inner.binary_search_by_key(&key, |(key, _)| *key) } -- 2.39.5 From 0097d1819eecdb83df98f1409fdacab1373daea1 Mon Sep 17 00:00:00 2001 From: Stu Black Date: Wed, 12 Nov 2025 17:03:56 -0500 Subject: [PATCH 14/24] Move functions on Bag which were only called by FFI code inline. --- wutil-rs/src/bag.rs | 39 +++++++++++---------------------------- 1 file changed, 11 insertions(+), 28 deletions(-) diff --git a/wutil-rs/src/bag.rs b/wutil-rs/src/bag.rs index 5115c91f..ffca050d 100644 --- a/wutil-rs/src/bag.rs +++ b/wutil-rs/src/bag.rs @@ -29,25 +29,6 @@ impl Bag { self.inner.binary_search_by_key(&key, |(key, _)| *key) } - /// Returns the index of `self.inner` where `key` is found. - fn find_index(&self, key: c_int) -> Option { - self.search(key).ok() - } - - /// Returns the value associated with `key`. - fn find_value(&self, key: c_int) -> Option> { - let i = self.find_index(key)?; - self.inner.get(i).copied().map(|(_, value)| value) - } - - /// Removes any value associated with `key`. - fn remove(&mut self, key: c_int) { - let Some(i) = self.find_index(key) else { - return; - }; - self.inner.remove(i); - } - /// Sets a value associated with `key` to `value`. Clobbers any extant value. fn set(&mut self, key: c_int, value: NonNull) { match self.search(key) { @@ -77,12 +58,13 @@ pub mod ffi { if bag.is_null() { return ptr::null_mut(); } - unsafe { - (*bag) - .find_value(key) - .map(|p| p.as_ptr()) - .unwrap_or(ptr::null_mut()) - } + let bag = unsafe { &mut *bag }; + bag + .search(key) + .ok() + .and_then(|index| bag.inner.get(index).copied()) + .map(|(_, value)| value.as_ptr()) + .unwrap_or(ptr::null_mut()) } /// Sets the value associated with `key` to `item`. If `item` is null, @@ -97,8 +79,9 @@ pub mod ffi { if let Some(item) = NonNull::new(item) { bag.set(key, item); } else { - bag.remove(key); - return; + if let Some(i) = bag.search(key).ok() { + bag.inner.remove(i); + } } } @@ -255,7 +238,7 @@ pub mod ffi { return ptr::null_mut(); } - if let Some(index) = bag.find_index(key) { + if let Some(index) = bag.search(key).ok() { *ptr = index as c_int; bag.inner[index].1.as_ptr() } else { -- 2.39.5 From dcd45f06775ef7a5ab2a559c81937970c185e369 Mon Sep 17 00:00:00 2001 From: Stu Black Date: Sat, 1 Nov 2025 15:50:17 -0400 Subject: [PATCH 15/24] Make some unused public WMNotification APIs private. `WMRetainNotification`, `WMGetDefaultNotificationQueue`, `WMDequeueNotificationMatching`, `WMEnqueueNotification`, and `WMEnqueueCoalesceNotification`, are only used in WINGs/notification.c. To reduce the API surface area that needs to be migrated to Rust, they are being made private. --- WINGs/WINGs/WUtil.h | 20 +------------------- WINGs/notification.c | 23 ++++++++++++++++++----- 2 files changed, 19 insertions(+), 24 deletions(-) diff --git a/WINGs/WINGs/WUtil.h b/WINGs/WINGs/WUtil.h index 060c1adb..224add25 100644 --- a/WINGs/WINGs/WUtil.h +++ b/WINGs/WINGs/WUtil.h @@ -596,15 +596,13 @@ WMTreeNode *WMFindInTreeWithDepthLimit(WMTreeNode * aTree, int (*match)(const WM /* Walk every node of aNode with `walk' */ void WMTreeWalk(WMTreeNode *aNode, WMTreeWalkProc * walk, void *data); -/* ---[ WINGs/data.c ]---------------------------------------------------- */ +/* ---[ WINGs/notification.c ]---------------------------------------------------- */ WMNotification* WMCreateNotification(const char *name, void *object, void *clientData); void WMReleaseNotification(WMNotification *notification); -WMNotification* WMRetainNotification(WMNotification *notification); - void* WMGetNotificationClientData(WMNotification *notification); void* WMGetNotificationObject(WMNotification *notification); @@ -624,24 +622,8 @@ void WMRemoveNotificationObserverWithName(void *observer, const char *name, void WMPostNotificationName(const char *name, void *object, void *clientData); -WMNotificationQueue* WMGetDefaultNotificationQueue(void); - WMNotificationQueue* WMCreateNotificationQueue(void); -void WMDequeueNotificationMatching(WMNotificationQueue *queue, - WMNotification *notification, - unsigned mask); - -void WMEnqueueNotification(WMNotificationQueue *queue, - WMNotification *notification, - WMPostingStyle postingStyle); - -void WMEnqueueCoalesceNotification(WMNotificationQueue *queue, - WMNotification *notification, - WMPostingStyle postingStyle, - unsigned coalesceMask); - - /* Property Lists handling */ /* ---[ WINGs/proplist.c ]------------------------------------------------ */ diff --git a/WINGs/notification.c b/WINGs/notification.c index d43fef54..7cfaac21 100644 --- a/WINGs/notification.c +++ b/WINGs/notification.c @@ -16,6 +16,19 @@ typedef struct W_Notification { } Notification; +static WMNotification* WMRetainNotification(WMNotification *notification); +static WMNotificationQueue* WMGetDefaultNotificationQueue(void); +static void WMDequeueNotificationMatching(WMNotificationQueue *queue, + WMNotification *notification, + unsigned mask); +static void WMEnqueueNotification(WMNotificationQueue *queue, + WMNotification *notification, + WMPostingStyle postingStyle); +static void WMEnqueueCoalesceNotification(WMNotificationQueue *queue, + WMNotification *notification, + WMPostingStyle postingStyle, + unsigned coalesceMask); + const char *WMGetNotificationName(WMNotification * notification) { return notification->name; @@ -53,7 +66,7 @@ void WMReleaseNotification(WMNotification * notification) } } -WMNotification *WMRetainNotification(WMNotification * notification) +static WMNotification *WMRetainNotification(WMNotification * notification) { notification->refCount++; @@ -365,7 +378,7 @@ static WMNotificationQueue *notificationQueueList = NULL; /* default queue */ static WMNotificationQueue *notificationQueue = NULL; -WMNotificationQueue *WMGetDefaultNotificationQueue(void) +static WMNotificationQueue *WMGetDefaultNotificationQueue(void) { if (!notificationQueue) notificationQueue = WMCreateNotificationQueue(); @@ -387,7 +400,7 @@ WMNotificationQueue *WMCreateNotificationQueue(void) return queue; } -void WMEnqueueNotification(WMNotificationQueue * queue, WMNotification * notification, WMPostingStyle postingStyle) +static void WMEnqueueNotification(WMNotificationQueue * queue, WMNotification * notification, WMPostingStyle postingStyle) { WMEnqueueCoalesceNotification(queue, notification, postingStyle, WNCOnName | WNCOnSender); } @@ -413,7 +426,7 @@ static int matchName(const void *item, const void *cdata) #undef NOTIF #undef ITEM -void WMDequeueNotificationMatching(WMNotificationQueue * queue, WMNotification * notification, unsigned mask) +static void WMDequeueNotificationMatching(WMNotificationQueue * queue, WMNotification * notification, unsigned mask) { WMMatchDataProc *matchFunc; @@ -430,7 +443,7 @@ void WMDequeueNotificationMatching(WMNotificationQueue * queue, WMNotification * WMRemoveFromArrayMatching(queue->idleQueue, matchFunc, notification); } -void +static void WMEnqueueCoalesceNotification(WMNotificationQueue * queue, WMNotification * notification, WMPostingStyle postingStyle, unsigned coalesceMask) { -- 2.39.5 From 0c4d78a53d2f1fcc7b4bce3d452306492af598ae Mon Sep 17 00:00:00 2001 From: Stu Black Date: Sun, 2 Nov 2025 14:25:49 -0500 Subject: [PATCH 16/24] Drop unused macro (dead code) that invokes WMCreateNotification. --- WINGs/wtext.c | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/WINGs/wtext.c b/WINGs/wtext.c index 61899ed8..c16773ff 100644 --- a/WINGs/wtext.c +++ b/WINGs/wtext.c @@ -166,16 +166,6 @@ typedef struct W_Text { WMArray *xdndDestinationTypes; } Text; -/* not used */ -#if 0 -#define NOTIFY(T,C,N,A) {\ - WMNotification *notif = WMCreateNotification(N,T,A);\ - if ((T)->delegate && (T)->delegate->C)\ - (*(T)->delegate->C)((T)->delegate,notif);\ - WMPostNotification(notif);\ - WMReleaseNotification(notif);} -#endif - #define TYPETEXT 0 #if 0 -- 2.39.5 From adb967ab1543265a91ae38090bc2171f4b83c219 Mon Sep 17 00:00:00 2001 From: Stu Black Date: Sun, 2 Nov 2025 15:59:55 -0500 Subject: [PATCH 17/24] Drop dead notification queue code. This appears to have been used by now-defunct support for network connections (WMConnection). No live code instantiates a notification queue or pushes/dequeues notifications from a notification queue. (The global NotificationCenter in WINGs/notification.c is still in use, so it is not going anywhere in this commit.) --- WINGs/WINGs/WINGsP.h.in | 4 -- WINGs/WINGs/WUtil.h | 2 - WINGs/handlers.c | 12 ---- WINGs/notification.c | 141 ---------------------------------------- 4 files changed, 159 deletions(-) diff --git a/WINGs/WINGs/WINGsP.h.in b/WINGs/WINGs/WINGsP.h.in index 2c36b269..a0020989 100644 --- a/WINGs/WINGs/WINGsP.h.in +++ b/WINGs/WINGs/WINGsP.h.in @@ -378,10 +378,6 @@ void W_InitNotificationCenter(void); void W_ReleaseNotificationCenter(void); -void W_FlushASAPNotificationQueue(void); - -void W_FlushIdleNotificationQueue(void); - /* ---[ selection.c ]----------------------------------------------------- */ diff --git a/WINGs/WINGs/WUtil.h b/WINGs/WINGs/WUtil.h index 224add25..aec32d48 100644 --- a/WINGs/WINGs/WUtil.h +++ b/WINGs/WINGs/WUtil.h @@ -622,8 +622,6 @@ void WMRemoveNotificationObserverWithName(void *observer, const char *name, void WMPostNotificationName(const char *name, void *object, void *clientData); -WMNotificationQueue* WMCreateNotificationQueue(void); - /* Property Lists handling */ /* ---[ WINGs/proplist.c ]------------------------------------------------ */ diff --git a/WINGs/handlers.c b/WINGs/handlers.c index c937aaa1..74f4fdd1 100644 --- a/WINGs/handlers.c +++ b/WINGs/handlers.c @@ -274,7 +274,6 @@ Bool W_CheckIdleHandlers(void) WMArrayIterator iter; if (!idleHandler || WMGetArrayItemCount(idleHandler) == 0) { - W_FlushIdleNotificationQueue(); /* make sure an observer in queue didn't added an idle handler */ return (idleHandler != NULL && WMGetArrayItemCount(idleHandler) > 0); } @@ -292,8 +291,6 @@ Bool W_CheckIdleHandlers(void) WMFreeArray(handlerCopy); - W_FlushIdleNotificationQueue(); - /* this is not necesarrily False, because one handler can re-add itself */ return (WMGetArrayItemCount(idleHandler) > 0); } @@ -304,7 +301,6 @@ void W_CheckTimerHandlers(void) struct timeval now; if (!timerHandler) { - W_FlushASAPNotificationQueue(); return; } @@ -331,8 +327,6 @@ void W_CheckTimerHandlers(void) wfree(handler); } } - - W_FlushASAPNotificationQueue(); } /* @@ -384,7 +378,6 @@ Bool W_HandleInputEvents(Bool waitForInput, int inputfd) nfds = 0; if (!extrafd && nfds == 0) { - W_FlushASAPNotificationQueue(); return False; } @@ -461,8 +454,6 @@ Bool W_HandleInputEvents(Bool waitForInput, int inputfd) wfree(fds); - W_FlushASAPNotificationQueue(); - return (count > 0); #else #ifdef HAVE_SELECT @@ -479,7 +470,6 @@ Bool W_HandleInputEvents(Bool waitForInput, int inputfd) nfds = 0; if (inputfd < 0 && nfds == 0) { - W_FlushASAPNotificationQueue(); return False; } @@ -556,8 +546,6 @@ Bool W_HandleInputEvents(Bool waitForInput, int inputfd) WMFreeArray(handlerCopy); } - W_FlushASAPNotificationQueue(); - return (count > 0); #else /* not HAVE_SELECT, not HAVE_POLL */ # error Neither select nor poll. You lose. diff --git a/WINGs/notification.c b/WINGs/notification.c index 7cfaac21..38b92c1a 100644 --- a/WINGs/notification.c +++ b/WINGs/notification.c @@ -17,17 +17,6 @@ typedef struct W_Notification { static WMNotification* WMRetainNotification(WMNotification *notification); -static WMNotificationQueue* WMGetDefaultNotificationQueue(void); -static void WMDequeueNotificationMatching(WMNotificationQueue *queue, - WMNotification *notification, - unsigned mask); -static void WMEnqueueNotification(WMNotificationQueue *queue, - WMNotification *notification, - WMPostingStyle postingStyle); -static void WMEnqueueCoalesceNotification(WMNotificationQueue *queue, - WMNotification *notification, - WMPostingStyle postingStyle, - unsigned coalesceMask); const char *WMGetNotificationName(WMNotification * notification) { @@ -363,133 +352,3 @@ void WMPostNotificationName(const char *name, void *object, void *clientData) WMReleaseNotification(notification); } - -/**************** Notification Queues ****************/ - -typedef struct W_NotificationQueue { - WMArray *asapQueue; - WMArray *idleQueue; - - struct W_NotificationQueue *next; -} NotificationQueue; - -static WMNotificationQueue *notificationQueueList = NULL; - -/* default queue */ -static WMNotificationQueue *notificationQueue = NULL; - -static WMNotificationQueue *WMGetDefaultNotificationQueue(void) -{ - if (!notificationQueue) - notificationQueue = WMCreateNotificationQueue(); - - return notificationQueue; -} - -WMNotificationQueue *WMCreateNotificationQueue(void) -{ - NotificationQueue *queue; - - queue = wmalloc(sizeof(NotificationQueue)); - queue->asapQueue = WMCreateArrayWithDestructor(8, (WMFreeDataProc *) WMReleaseNotification); - queue->idleQueue = WMCreateArrayWithDestructor(8, (WMFreeDataProc *) WMReleaseNotification); - queue->next = notificationQueueList; - - notificationQueueList = queue; - - return queue; -} - -static void WMEnqueueNotification(WMNotificationQueue * queue, WMNotification * notification, WMPostingStyle postingStyle) -{ - WMEnqueueCoalesceNotification(queue, notification, postingStyle, WNCOnName | WNCOnSender); -} - -#define NOTIF ((WMNotification*)cdata) -#define ITEM ((WMNotification*)item) - -static int matchSenderAndName(const void *item, const void *cdata) -{ - return (NOTIF->object == ITEM->object && strcmp(NOTIF->name, ITEM->name) == 0); -} - -static int matchSender(const void *item, const void *cdata) -{ - return (NOTIF->object == ITEM->object); -} - -static int matchName(const void *item, const void *cdata) -{ - return (strcmp(NOTIF->name, ITEM->name) == 0); -} - -#undef NOTIF -#undef ITEM - -static void WMDequeueNotificationMatching(WMNotificationQueue * queue, WMNotification * notification, unsigned mask) -{ - WMMatchDataProc *matchFunc; - - if ((mask & WNCOnName) && (mask & WNCOnSender)) - matchFunc = matchSenderAndName; - else if (mask & WNCOnName) - matchFunc = matchName; - else if (mask & WNCOnSender) - matchFunc = matchSender; - else - return; - - WMRemoveFromArrayMatching(queue->asapQueue, matchFunc, notification); - WMRemoveFromArrayMatching(queue->idleQueue, matchFunc, notification); -} - -static void -WMEnqueueCoalesceNotification(WMNotificationQueue * queue, - WMNotification * notification, WMPostingStyle postingStyle, unsigned coalesceMask) -{ - if (coalesceMask != WNCNone) - WMDequeueNotificationMatching(queue, notification, coalesceMask); - - switch (postingStyle) { - case WMPostNow: - WMPostNotification(notification); - WMReleaseNotification(notification); - break; - - case WMPostASAP: - WMAddToArray(queue->asapQueue, notification); - break; - - case WMPostWhenIdle: - WMAddToArray(queue->idleQueue, notification); - break; - } -} - -void W_FlushASAPNotificationQueue(void) -{ - WMNotificationQueue *queue = notificationQueueList; - - while (queue) { - while (WMGetArrayItemCount(queue->asapQueue)) { - WMPostNotification(WMGetFromArray(queue->asapQueue, 0)); - WMDeleteFromArray(queue->asapQueue, 0); - } - - queue = queue->next; - } -} - -void W_FlushIdleNotificationQueue(void) -{ - WMNotificationQueue *queue = notificationQueueList; - - while (queue) { - while (WMGetArrayItemCount(queue->idleQueue)) { - WMPostNotification(WMGetFromArray(queue->idleQueue, 0)); - WMDeleteFromArray(queue->idleQueue, 0); - } - - queue = queue->next; - } -} -- 2.39.5 From 5847e9d68f87a82f979ec7ad4d5d5d2eafa8857c Mon Sep 17 00:00:00 2001 From: Stu Black Date: Sat, 8 Nov 2025 12:26:57 -0500 Subject: [PATCH 18/24] Ditch the notification function WMRemoveNotificationObserverWithName. This reduces the notification API in a way that is helpful for rewriting in Rust. This function is only used in one place, and the object that is being deregistered is free'd immediately after WMRemoveNotificationObserverWithName is called, so it should be safe just to use WMRemoveNotificationObserver (since I hope it's an error to keep a notification to a free'd object registered). --- WINGs/WINGs/WUtil.h | 3 -- WINGs/notification.c | 76 -------------------------------------------- WINGs/wbrowser.c | 2 +- 3 files changed, 1 insertion(+), 80 deletions(-) diff --git a/WINGs/WINGs/WUtil.h b/WINGs/WINGs/WUtil.h index aec32d48..45557e84 100644 --- a/WINGs/WINGs/WUtil.h +++ b/WINGs/WINGs/WUtil.h @@ -617,9 +617,6 @@ void WMPostNotification(WMNotification *notification); void WMRemoveNotificationObserver(void *observer); -void WMRemoveNotificationObserverWithName(void *observer, const char *name, - void *object); - void WMPostNotificationName(const char *name, void *object, void *clientData); /* Property Lists handling */ diff --git a/WINGs/notification.c b/WINGs/notification.c index 38b92c1a..335189d7 100644 --- a/WINGs/notification.c +++ b/WINGs/notification.c @@ -266,82 +266,6 @@ void WMRemoveNotificationObserver(void *observer) WMHashRemove(notificationCenter->observerTable, observer); } -void WMRemoveNotificationObserverWithName(void *observer, const char *name, void *object) -{ - NotificationObserver *orec, *tmp, *rec; - NotificationObserver *newList = NULL; - - /* get the list of actions the observer is doing */ - orec = (NotificationObserver *) WMHashGet(notificationCenter->observerTable, observer); - - WMHashRemove(notificationCenter->observerTable, observer); - - /* rebuild the list of actions for the observer */ - - while (orec) { - tmp = orec->nextAction; - if (orec->name == name && orec->object == object) { - if (!name && !object) { - if (notificationCenter->nilList == orec) - notificationCenter->nilList = orec->next; - } else if (!name) { - rec = - (NotificationObserver *) WMHashGet(notificationCenter->objectTable, - orec->object); - if (rec == orec) { - assert(rec->prev == NULL); - /* replace table entry */ - if (orec->next) { - WMHashInsert(notificationCenter->objectTable, - orec->object, orec->next); - } else { - WMHashRemove(notificationCenter->objectTable, orec->object); - } - } - } else { - rec = (NotificationObserver *) WMHashGet(notificationCenter->nameTable, - orec->name); - if (rec == orec) { - assert(rec->prev == NULL); - /* replace table entry */ - if (orec->next) { - WMHashInsert(notificationCenter->nameTable, - orec->name, orec->next); - } else { - WMHashRemove(notificationCenter->nameTable, orec->name); - } - } - } - - if (orec->prev) - orec->prev->next = orec->next; - if (orec->next) - orec->next->prev = orec->prev; - wfree(orec); - } else { - /* append this action in the new action list */ - orec->nextAction = NULL; - if (!newList) { - newList = orec; - } else { - NotificationObserver *p; - - p = newList; - while (p->nextAction) { - p = p->nextAction; - } - p->nextAction = orec; - } - } - orec = tmp; - } - - /* reinsert the list to the table */ - if (newList) { - WMHashInsert(notificationCenter->observerTable, observer, newList); - } -} - void WMPostNotificationName(const char *name, void *object, void *clientData) { WMNotification *notification; diff --git a/WINGs/wbrowser.c b/WINGs/wbrowser.c index b25bc8c1..41b43047 100644 --- a/WINGs/wbrowser.c +++ b/WINGs/wbrowser.c @@ -314,7 +314,7 @@ static void removeColumn(WMBrowser * bPtr, int column) wfree(bPtr->titles[i]); bPtr->titles[i] = NULL; } - WMRemoveNotificationObserverWithName(bPtr, WMListSelectionDidChangeNotification, bPtr->columns[i]); + WMRemoveNotificationObserver(bPtr->columns[i]); WMDestroyWidget(bPtr->columns[i]); bPtr->columns[i] = NULL; } -- 2.39.5 From d88d626fbe3bb5def76e778c731816160ade84a4 Mon Sep 17 00:00:00 2001 From: Stu Black Date: Wed, 12 Nov 2025 12:15:13 -0500 Subject: [PATCH 19/24] Remove direct creation and posting of notifications from WINGs. We can reduce the WMNotification API surface area further by getting rid of WMCreateNotification and WMPostNotification. WMNotification remains a first-class object, but it is no longer possible for client code to create a notification object directly. Notifications must now be posted through WMPostNotificationName, which handles notification creation and destruction on its own. This will simplify the notification lifecycle and make the Rust rewrite simpler. (Notifications no longer need to be reference-counted, heap-allocated objects that might be saved somewhere after they are dispatched.) WTextField code which reused the WMNotification struct has been modified to take parameters of the correct type directly, instead of through a WMNotification's void* data field. --- WINGs/WINGs/WINGs.h | 17 +++++------ WINGs/WINGs/WUtil.h | 4 --- WINGs/wtextfield.c | 69 ++++++++++++++++++++++++------------------- WPrefs.app/editmenu.c | 5 +--- 4 files changed, 47 insertions(+), 48 deletions(-) diff --git a/WINGs/WINGs/WINGs.h b/WINGs/WINGs/WINGs.h index b6192bdb..2d2b3a8e 100644 --- a/WINGs/WINGs/WINGs.h +++ b/WINGs/WINGs/WINGs.h @@ -233,7 +233,7 @@ typedef enum { /* text movement types */ -enum { +typedef enum { WMIllegalTextMovement, WMReturnTextMovement, WMEscapeTextMovement, @@ -243,13 +243,13 @@ enum { WMRightTextMovement, WMUpTextMovement, WMDownTextMovement -}; +} WMTextMovementType; /* text field special events */ -enum { +typedef enum { WMInsertTextEvent, WMDeleteTextEvent -}; +} WMTextFieldSpecialEventType; enum { @@ -533,14 +533,11 @@ typedef struct WMBrowserDelegate { typedef struct WMTextFieldDelegate { void *data; - void (*didBeginEditing)(struct WMTextFieldDelegate *self, - WMNotification *notif); + void (*didBeginEditing)(struct WMTextFieldDelegate *self, WMTextMovementType reason); - void (*didChange)(struct WMTextFieldDelegate *self, - WMNotification *notif); + void (*didChange)(struct WMTextFieldDelegate *self, WMTextFieldSpecialEventType reason); - void (*didEndEditing)(struct WMTextFieldDelegate *self, - WMNotification *notif); + void (*didEndEditing)(struct WMTextFieldDelegate *self, WMTextMovementType reason); Bool (*shouldBeginEditing)(struct WMTextFieldDelegate *self, WMTextField *tPtr); diff --git a/WINGs/WINGs/WUtil.h b/WINGs/WINGs/WUtil.h index 45557e84..f290f187 100644 --- a/WINGs/WINGs/WUtil.h +++ b/WINGs/WINGs/WUtil.h @@ -599,8 +599,6 @@ void WMTreeWalk(WMTreeNode *aNode, WMTreeWalkProc * walk, void *data); /* ---[ WINGs/notification.c ]---------------------------------------------------- */ -WMNotification* WMCreateNotification(const char *name, void *object, void *clientData); - void WMReleaseNotification(WMNotification *notification); void* WMGetNotificationClientData(WMNotification *notification); @@ -613,8 +611,6 @@ const char* WMGetNotificationName(WMNotification *notification); void WMAddNotificationObserver(WMNotificationObserverAction *observerAction, void *observer, const char *name, void *object); -void WMPostNotification(WMNotification *notification); - void WMRemoveNotificationObserver(void *observer); void WMPostNotificationName(const char *name, void *object, void *clientData); diff --git a/WINGs/wtextfield.c b/WINGs/wtextfield.c index f8cf253e..b26ca5eb 100644 --- a/WINGs/wtextfield.c +++ b/WINGs/wtextfield.c @@ -68,12 +68,6 @@ typedef struct W_TextField { } flags; } TextField; -#define NOTIFY(T,C,N,A) { WMNotification *notif = WMCreateNotification(N,T,A);\ - if ((T)->delegate && (T)->delegate->C)\ - (*(T)->delegate->C)((T)->delegate,notif);\ - WMPostNotification(notif);\ - WMReleaseNotification(notif);} - #define MIN_TEXT_BUFFER 2 #define TEXT_BUFFER_INCR 8 @@ -906,7 +900,10 @@ static void handleEvents(XEvent * event, void *data) paintTextField(tPtr); - NOTIFY(tPtr, didBeginEditing, WMTextDidBeginEditingNotification, NULL); + if (tPtr->delegate && tPtr->delegate->didBeginEditing) { + (*tPtr->delegate->didBeginEditing)(tPtr->delegate, 0); + } + WMPostNotificationName(WMTextDidBeginEditingNotification, tPtr, NULL); tPtr->flags.notIllegalMovement = 0; break; @@ -921,8 +918,10 @@ static void handleEvents(XEvent * event, void *data) paintTextField(tPtr); if (!tPtr->flags.notIllegalMovement) { - NOTIFY(tPtr, didEndEditing, WMTextDidEndEditingNotification, - (void *)WMIllegalTextMovement); + if (tPtr->delegate && tPtr->delegate->didEndEditing) { + (*tPtr->delegate->didEndEditing)(tPtr->delegate, WMIllegalTextMovement); + } + WMPostNotificationName(WMTextDidEndEditingNotification, tPtr, (void *)WMIllegalTextMovement); } break; @@ -943,7 +942,8 @@ static void handleTextFieldKeyPress(TextField * tPtr, XEvent * event) char buffer[64]; KeySym ksym; const char *textEvent = NULL; - void *data = NULL; + WMTextMovementType movement_type; + WMTextFieldSpecialEventType special_field_event_type; int count, refresh = 0; int control_pressed = 0; int cancelSelection = 1; @@ -975,14 +975,14 @@ static void handleTextFieldKeyPress(TextField * tPtr, XEvent * event) tPtr->view->prevFocusChain); tPtr->flags.notIllegalMovement = 1; } - data = (void *)WMBacktabTextMovement; + movement_type = WMBacktabTextMovement; } else { if (tPtr->view->nextFocusChain) { W_SetFocusOfTopLevel(W_TopLevelOfView(tPtr->view), tPtr->view->nextFocusChain); tPtr->flags.notIllegalMovement = 1; } - data = (void *)WMTabTextMovement; + movement_type = WMTabTextMovement; } textEvent = WMTextDidEndEditingNotification; @@ -994,7 +994,7 @@ static void handleTextFieldKeyPress(TextField * tPtr, XEvent * event) case XK_Escape: if (!modified) { - data = (void *)WMEscapeTextMovement; + movement_type = WMEscapeTextMovement; textEvent = WMTextDidEndEditingNotification; relay = False; @@ -1008,7 +1008,7 @@ static void handleTextFieldKeyPress(TextField * tPtr, XEvent * event) /* FALLTHRU */ case XK_Return: if (!modified) { - data = (void *)WMReturnTextMovement; + movement_type = WMReturnTextMovement; textEvent = WMTextDidEndEditingNotification; relay = False; @@ -1165,7 +1165,7 @@ static void handleTextFieldKeyPress(TextField * tPtr, XEvent * event) if (!modified) { if (tPtr->selection.count) { WMDeleteTextFieldRange(tPtr, tPtr->selection); - data = (void *)WMDeleteTextEvent; + special_field_event_type = WMDeleteTextEvent; textEvent = WMTextDidChangeNotification; } else if (tPtr->cursorPosition > 0) { int i = oneUTF8CharBackward(&tPtr->text[tPtr->cursorPosition], @@ -1174,7 +1174,7 @@ static void handleTextFieldKeyPress(TextField * tPtr, XEvent * event) range.position = tPtr->cursorPosition + i; range.count = -i; WMDeleteTextFieldRange(tPtr, range); - data = (void *)WMDeleteTextEvent; + special_field_event_type = WMDeleteTextEvent; textEvent = WMTextDidChangeNotification; } @@ -1197,7 +1197,7 @@ static void handleTextFieldKeyPress(TextField * tPtr, XEvent * event) if (!modified) { if (tPtr->selection.count) { WMDeleteTextFieldRange(tPtr, tPtr->selection); - data = (void *)WMDeleteTextEvent; + special_field_event_type = WMDeleteTextEvent; textEvent = WMTextDidChangeNotification; } else if (tPtr->cursorPosition < tPtr->textLen) { WMRange range; @@ -1205,7 +1205,7 @@ static void handleTextFieldKeyPress(TextField * tPtr, XEvent * event) range.count = oneUTF8CharForward(&tPtr->text[tPtr->cursorPosition], tPtr->textLen - tPtr->cursorPosition); WMDeleteTextFieldRange(tPtr, range); - data = (void *)WMDeleteTextEvent; + special_field_event_type = WMDeleteTextEvent; textEvent = WMTextDidChangeNotification; } @@ -1220,7 +1220,7 @@ static void handleTextFieldKeyPress(TextField * tPtr, XEvent * event) if (tPtr->selection.count) WMDeleteTextFieldRange(tPtr, tPtr->selection); WMInsertTextFieldText(tPtr, buffer, tPtr->cursorPosition); - data = (void *)WMInsertTextEvent; + special_field_event_type = WMInsertTextEvent; textEvent = WMTextDidChangeNotification; relay = False; @@ -1255,21 +1255,22 @@ static void handleTextFieldKeyPress(TextField * tPtr, XEvent * event) /*printf("(%d,%d)\n", tPtr->selection.position, tPtr->selection.count); */ if (textEvent) { - WMNotification *notif = WMCreateNotification(textEvent, tPtr, data); - if (tPtr->delegate) { if (textEvent == WMTextDidBeginEditingNotification && tPtr->delegate->didBeginEditing) - (*tPtr->delegate->didBeginEditing) (tPtr->delegate, notif); + (*tPtr->delegate->didBeginEditing) (tPtr->delegate, movement_type); else if (textEvent == WMTextDidEndEditingNotification && tPtr->delegate->didEndEditing) - (*tPtr->delegate->didEndEditing) (tPtr->delegate, notif); + (*tPtr->delegate->didEndEditing) (tPtr->delegate, movement_type); else if (textEvent == WMTextDidChangeNotification && tPtr->delegate->didChange) - (*tPtr->delegate->didChange) (tPtr->delegate, notif); + (*tPtr->delegate->didChange) (tPtr->delegate, special_field_event_type); } - WMPostNotification(notif); - WMReleaseNotification(notif); + if (textEvent == WMTextDidBeginEditingNotification || textEvent == WMTextDidEndEditingNotification) { + WMPostNotificationName(textEvent, tPtr, (void *)movement_type); + } else if (textEvent == WMTextDidChangeNotification) { + WMPostNotificationName(textEvent, tPtr, (void *)special_field_event_type); + } } if (refresh) @@ -1345,7 +1346,10 @@ static void pasteText(WMView * view, Atom selection, Atom target, Time timestamp str = (char *)WMDataBytes(data); WMInsertTextFieldText(tPtr, str, tPtr->cursorPosition); - NOTIFY(tPtr, didChange, WMTextDidChangeNotification, (void *)WMInsertTextEvent); + if (tPtr->delegate && tPtr->delegate->didChange) { + (*tPtr->delegate->didChange)(tPtr->delegate, WMInsertTextEvent); + } + WMPostNotificationName(WMTextDidChangeNotification, tPtr, (void *)WMInsertTextEvent); } else { int n; @@ -1355,7 +1359,10 @@ static void pasteText(WMView * view, Atom selection, Atom target, Time timestamp str[n] = 0; WMInsertTextFieldText(tPtr, str, tPtr->cursorPosition); XFree(str); - NOTIFY(tPtr, didChange, WMTextDidChangeNotification, (void *)WMInsertTextEvent); + if (tPtr->delegate && tPtr->delegate->didChange) { + (*tPtr->delegate->didChange)(tPtr->delegate, WMInsertTextEvent); + } + WMPostNotificationName(WMTextDidChangeNotification, tPtr, (void *)WMInsertTextEvent); } } } @@ -1477,8 +1484,10 @@ static void handleTextFieldActionEvents(XEvent * event, void *data) text[n] = 0; WMInsertTextFieldText(tPtr, text, tPtr->cursorPosition); XFree(text); - NOTIFY(tPtr, didChange, WMTextDidChangeNotification, - (void *)WMInsertTextEvent); + if (tPtr->delegate && tPtr->delegate->didChange) { + (*tPtr->delegate->didChange)(tPtr->delegate, WMInsertTextEvent); + } + WMPostNotificationName(WMTextDidChangeNotification, tPtr, (void *)WMInsertTextEvent); } } else { tPtr->flags.waitingSelection = 1; diff --git a/WPrefs.app/editmenu.c b/WPrefs.app/editmenu.c index c84dc08a..4eb9e8f1 100644 --- a/WPrefs.app/editmenu.c +++ b/WPrefs.app/editmenu.c @@ -824,18 +824,15 @@ static void stopEditItem(WEditMenu * menu, Bool apply) menu->flags.isEditing = 0; } -static void textEndedEditing(struct WMTextFieldDelegate *self, WMNotification * notif) +static void textEndedEditing(struct WMTextFieldDelegate *self, WMTextMovementType reason) { WEditMenu *menu = (WEditMenu *) self->data; - uintptr_t reason; int i; WEditMenuItem *item; if (!menu->flags.isEditing) return; - reason = (uintptr_t)WMGetNotificationClientData(notif); - switch (reason) { case WMEscapeTextMovement: stopEditItem(menu, False); -- 2.39.5 From b7f765e3f6d2debd0ee71589602415fae1df762d Mon Sep 17 00:00:00 2001 From: Stu Black Date: Wed, 12 Nov 2025 16:33:48 -0500 Subject: [PATCH 20/24] Rewrite WINGs/notification.c in Rust. --- WINGs/Makefile.am | 1 - WINGs/WINGs/WUtil.h | 2 - WINGs/notification.c | 278 -------------------------- WINGs/wapplication.c | 5 +- wutil-rs/Makefile.am | 1 + wutil-rs/src/lib.rs | 1 + wutil-rs/src/notification.rs | 368 +++++++++++++++++++++++++++++++++++ 7 files changed, 371 insertions(+), 285 deletions(-) delete mode 100644 WINGs/notification.c create mode 100644 wutil-rs/src/notification.rs diff --git a/WINGs/Makefile.am b/WINGs/Makefile.am index 74b3ffea..8a1a839e 100644 --- a/WINGs/Makefile.am +++ b/WINGs/Makefile.am @@ -73,7 +73,6 @@ libWUtil_la_SOURCES = \ menuparser.h \ menuparser_macros.c \ misc.c \ - notification.c \ proplist.c \ userdefaults.c \ userdefaults.h \ diff --git a/WINGs/WINGs/WUtil.h b/WINGs/WINGs/WUtil.h index f290f187..d4c499f1 100644 --- a/WINGs/WINGs/WUtil.h +++ b/WINGs/WINGs/WUtil.h @@ -599,8 +599,6 @@ void WMTreeWalk(WMTreeNode *aNode, WMTreeWalkProc * walk, void *data); /* ---[ WINGs/notification.c ]---------------------------------------------------- */ -void WMReleaseNotification(WMNotification *notification); - void* WMGetNotificationClientData(WMNotification *notification); void* WMGetNotificationObject(WMNotification *notification); diff --git a/WINGs/notification.c b/WINGs/notification.c deleted file mode 100644 index 335189d7..00000000 --- a/WINGs/notification.c +++ /dev/null @@ -1,278 +0,0 @@ - -#include -#include -#include -#include - -#include "WUtil.h" -#include "WINGsP.h" - - -typedef struct W_Notification { - const char *name; - void *object; - void *clientData; - int refCount; -} Notification; - - -static WMNotification* WMRetainNotification(WMNotification *notification); - -const char *WMGetNotificationName(WMNotification * notification) -{ - return notification->name; -} - -void *WMGetNotificationObject(WMNotification * notification) -{ - return notification->object; -} - -void *WMGetNotificationClientData(WMNotification * notification) -{ - return notification->clientData; -} - -WMNotification *WMCreateNotification(const char *name, void *object, void *clientData) -{ - Notification *nPtr; - - nPtr = wmalloc(sizeof(Notification)); - nPtr->name = name; - nPtr->object = object; - nPtr->clientData = clientData; - nPtr->refCount = 1; - - return nPtr; -} - -void WMReleaseNotification(WMNotification * notification) -{ - notification->refCount--; - - if (notification->refCount < 1) { - wfree(notification); - } -} - -static WMNotification *WMRetainNotification(WMNotification * notification) -{ - notification->refCount++; - - return notification; -} - -/***************** Notification Center *****************/ - -typedef struct NotificationObserver { - WMNotificationObserverAction *observerAction; - void *observer; - - const char *name; - void *object; - - struct NotificationObserver *prev; /* for tables */ - struct NotificationObserver *next; - struct NotificationObserver *nextAction; /* for observerTable */ -} NotificationObserver; - -typedef struct W_NotificationCenter { - WMHashTable *nameTable; /* names -> observer lists */ - WMHashTable *objectTable; /* object -> observer lists */ - NotificationObserver *nilList; /* obervers that catch everything */ - - WMHashTable *observerTable; /* observer -> NotificationObserver */ -} NotificationCenter; - -/* default (and only) center */ -static NotificationCenter *notificationCenter = NULL; - -void W_InitNotificationCenter(void) -{ - notificationCenter = wmalloc(sizeof(NotificationCenter)); - notificationCenter->nameTable = WMCreateStringHashTable(); - notificationCenter->objectTable = WMCreateIdentityHashTable(); - notificationCenter->nilList = NULL; - notificationCenter->observerTable = WMCreateIdentityHashTable(); -} - -void W_ReleaseNotificationCenter(void) -{ - if (notificationCenter) { - if (notificationCenter->nameTable) - WMFreeHashTable(notificationCenter->nameTable); - if (notificationCenter->objectTable) - WMFreeHashTable(notificationCenter->objectTable); - if (notificationCenter->observerTable) - WMFreeHashTable(notificationCenter->observerTable); - - wfree(notificationCenter); - notificationCenter = NULL; - } -} - -void -WMAddNotificationObserver(WMNotificationObserverAction * observerAction, - void *observer, const char *name, void *object) -{ - NotificationObserver *oRec, *rec; - - oRec = wmalloc(sizeof(NotificationObserver)); - oRec->observerAction = observerAction; - oRec->observer = observer; - oRec->name = name; - oRec->object = object; - oRec->next = NULL; - oRec->prev = NULL; - - /* put this action in the list of actions for this observer */ - rec = (NotificationObserver *) WMHashInsert(notificationCenter->observerTable, observer, oRec); - - if (rec) { - /* if this is not the first action for the observer */ - oRec->nextAction = rec; - } else { - oRec->nextAction = NULL; - } - - if (!name && !object) { - /* catch-all */ - oRec->next = notificationCenter->nilList; - if (notificationCenter->nilList) { - notificationCenter->nilList->prev = oRec; - } - notificationCenter->nilList = oRec; - } else if (!name) { - /* any message coming from object */ - rec = (NotificationObserver *) WMHashInsert(notificationCenter->objectTable, object, oRec); - oRec->next = rec; - if (rec) { - rec->prev = oRec; - } - } else { - /* name && (object || !object) */ - rec = (NotificationObserver *) WMHashInsert(notificationCenter->nameTable, name, oRec); - oRec->next = rec; - if (rec) { - rec->prev = oRec; - } - } -} - -void WMPostNotification(WMNotification * notification) -{ - NotificationObserver *orec, *tmp; - - WMRetainNotification(notification); - - /* tell the observers that want to know about a particular message */ - orec = (NotificationObserver *) WMHashGet(notificationCenter->nameTable, notification->name); - - while (orec) { - tmp = orec->next; - - if (!orec->object || !notification->object || orec->object == notification->object) { - /* tell the observer */ - if (orec->observerAction) { - (*orec->observerAction) (orec->observer, notification); - } - } - - orec = tmp; - } - - /* tell the observers that want to know about an object */ - orec = (NotificationObserver *) WMHashGet(notificationCenter->objectTable, notification->object); - - while (orec) { - tmp = orec->next; - - /* tell the observer */ - if (orec->observerAction) { - (*orec->observerAction) (orec->observer, notification); - } - orec = tmp; - } - - /* tell the catch all observers */ - orec = notificationCenter->nilList; - while (orec) { - tmp = orec->next; - - /* tell the observer */ - if (orec->observerAction) { - (*orec->observerAction) (orec->observer, notification); - } - orec = tmp; - } - - WMReleaseNotification(notification); -} - -void WMRemoveNotificationObserver(void *observer) -{ - NotificationObserver *orec, *tmp, *rec; - - /* get the list of actions the observer is doing */ - orec = (NotificationObserver *) WMHashGet(notificationCenter->observerTable, observer); - - /* - * FOREACH orec IN actionlist for observer - * DO - * remove from respective lists/tables - * free - * END - */ - while (orec) { - tmp = orec->nextAction; - - if (!orec->name && !orec->object) { - /* catch-all */ - if (notificationCenter->nilList == orec) - notificationCenter->nilList = orec->next; - } else if (!orec->name) { - /* any message coming from object */ - rec = (NotificationObserver *) WMHashGet(notificationCenter->objectTable, orec->object); - if (rec == orec) { - /* replace table entry */ - if (orec->next) { - WMHashInsert(notificationCenter->objectTable, orec->object, orec->next); - } else { - WMHashRemove(notificationCenter->objectTable, orec->object); - } - } - } else { - /* name && (object || !object) */ - rec = (NotificationObserver *) WMHashGet(notificationCenter->nameTable, orec->name); - if (rec == orec) { - /* replace table entry */ - if (orec->next) { - WMHashInsert(notificationCenter->nameTable, orec->name, orec->next); - } else { - WMHashRemove(notificationCenter->nameTable, orec->name); - } - } - } - if (orec->prev) - orec->prev->next = orec->next; - if (orec->next) - orec->next->prev = orec->prev; - - wfree(orec); - - orec = tmp; - } - - WMHashRemove(notificationCenter->observerTable, observer); -} - -void WMPostNotificationName(const char *name, void *object, void *clientData) -{ - WMNotification *notification; - - notification = WMCreateNotification(name, object, clientData); - - WMPostNotification(notification); - - WMReleaseNotification(notification); -} diff --git a/WINGs/wapplication.c b/WINGs/wapplication.c index 45070a22..312cbee9 100644 --- a/WINGs/wapplication.c +++ b/WINGs/wapplication.c @@ -44,9 +44,6 @@ void WMInitializeApplication(const char *applicationName, int *argc, char **argv WMApplication.argv[i] = wstrdup(argv[i]); } WMApplication.argv[i] = NULL; - - /* initialize notification center */ - W_InitNotificationCenter(); } void WMReleaseApplication(void) { @@ -60,7 +57,7 @@ void WMReleaseApplication(void) { */ w_save_defaults_changes(); - W_ReleaseNotificationCenter(); + W_ClearNotificationCenter(); if (WMApplication.applicationName) { wfree(WMApplication.applicationName); diff --git a/wutil-rs/Makefile.am b/wutil-rs/Makefile.am index e2ee2d06..ab11b9c9 100644 --- a/wutil-rs/Makefile.am +++ b/wutil-rs/Makefile.am @@ -10,6 +10,7 @@ RUST_SOURCES = \ src/hash_table.rs \ src/lib.rs \ src/memory.rs \ + src/notification.rs \ src/prop_list.rs \ src/string.rs src/tree.rs diff --git a/wutil-rs/src/lib.rs b/wutil-rs/src/lib.rs index ab94d66d..d2798268 100644 --- a/wutil-rs/src/lib.rs +++ b/wutil-rs/src/lib.rs @@ -5,6 +5,7 @@ pub mod defines; pub mod find_file; pub mod hash_table; pub mod memory; +pub mod notification; pub mod prop_list; pub mod string; pub mod tree; diff --git a/wutil-rs/src/notification.rs b/wutil-rs/src/notification.rs new file mode 100644 index 00000000..dc56c51e --- /dev/null +++ b/wutil-rs/src/notification.rs @@ -0,0 +1,368 @@ +use std::{ + collections::{hash_map::Entry, HashMap}, + ffi::{c_void, CStr}, + hash, + ptr::{self, NonNull}, + sync::{Mutex, OnceLock}, +}; + +/// Lightweight message from object to another. When a notification is sent, +/// registered [`Action`]s are called. +/// +/// Use [`ffi::WMAddNotificationObserver`] or [`NotificationCenter::register`] +/// to request notifications. +/// +/// ## Safety +/// +/// `Notification` encapsulates two data pointers. The Rust implementation +/// explicitly supports notifications across threads. To uphold Rust's safety +/// rules, `Notification`s must only be created with pointers that can be sent +/// across threads. The [`Sendable`] struct is provided to guarantee this. +/// +/// ## Rust rewrite notes +/// +/// This was originally a reference-counted structure, but it is so lightweight +/// (consisting of three pointers) that the Rust version is `Copy`. This +/// simplifies things --- each Rust recipient of a `Notification` owns it, and +/// there is no need to coordinate how it is cleaned up. +/// +/// In unported C code, a notificaton's `name` may be compared against a string +/// constant using pointer equality rather than string equality. This has +/// negative implications for sending notifications across the Rust/C +/// boundary. For the time being, notifications are generated and received by +/// code written in C, but care should be taken that notification names are +/// checked properly when porting notification registration in the future. (We +/// will ideally move to a different data type for identifying notifications, +/// too.) +#[derive(Clone, Copy)] +pub struct Notification { + name: &'static CStr, + /// The object that generated the notification. This may be `None` for + /// notifications that are about global state. + source: Option, + /// Optional side-channel data provided to notification listeners. + client_data: Option, +} + +/// Callback that notifies `observer` (which may be null) of `notification` (which won't be). +pub type Action = unsafe extern "C" fn(observer: *mut c_void, notification: *const Notification); + +/// Wraps a type-erased pointer (which it does not own) and marks it as `Send`. +/// +/// The `Send`-ability of the wrapped pointer must be guaranteed code that +/// instantiates a `Sendable`. +#[derive(Clone, Copy, Eq, PartialEq)] +pub struct Sendable { + ptr: NonNull, +} + +impl Sendable { + /// Creates a `Sendable` wrapping `ptr`. + /// + /// ## Safety + /// + /// `ptr` must be safe to send across threads. + pub unsafe fn new(ptr: NonNull) -> Self { + Sendable { ptr } + } +} + +impl hash::Hash for Sendable { + fn hash(&self, h: &mut H) { + h.write_usize(self.ptr.as_ptr().addr()); + } +} + +// Guaranteed by `Sendable::new`. +unsafe impl Send for Sendable {} + +#[derive(Default)] +pub struct NotificationCenter { + /// Notification subscriptions that match on name and source. + exact: HashMap<(&'static CStr, Sendable), Vec<(Option, Action)>>, + /// Notification subscriptions that match on name. + by_name: HashMap<&'static CStr, Vec<(Option, Action)>>, + /// Notification subscriptions that match on source. + by_source: HashMap, Action)>>, + /// Notification subscriptions that match all notifications. + universal: Vec<(Option, Action)>, +} + +// It is safe to send NotificationCenter across threads as long as the contract +// on Sendable is respected. +unsafe impl Send for NotificationCenter {} + +impl NotificationCenter { + /// Creates a new `NotificationCenter`. + pub fn new() -> Self { + Self::default() + } + + /// Provides access to the default, process-wide notification center. The + /// FFI C API uses this notification center. This is protected behind a + /// mutex that is held while `f` is run, so panicking inside of `f` should + /// be avoided. + pub fn with_global_default(f: impl FnOnce(&mut Self) -> R) -> R { + static INSTANCE: OnceLock> = OnceLock::new(); + f(&mut INSTANCE + .get_or_init(|| Mutex::new(NotificationCenter::new())) + .try_lock() + .unwrap()) + } + + /// Registers `action` to be invoked and invoked on `observer` when + /// notifications named `name` are fired from `source`. + pub fn register_exact( + &mut self, + name: &'static CStr, + source: Sendable, + observer: Option, + action: Action, + ) { + match self.exact.entry((name, source)) { + Entry::Occupied(mut o) => { + o.get_mut().push((observer, action)); + } + Entry::Vacant(v) => { + v.insert(vec![(observer, action)]); + } + } + } + + /// Registers `action` to be invoked on `observer` when notifications are + /// fired by `source` (regardless of the notification name). + pub fn register_by_source( + &mut self, + source: Sendable, + observer: Option, + action: Action, + ) { + match self.by_source.entry(source) { + Entry::Occupied(mut o) => { + o.get_mut().push((observer, action)); + } + Entry::Vacant(v) => { + v.insert(vec![(observer, action)]); + } + } + } + + /// Registers `action` to be invoked on `observer` for all notifications + /// named `name`. + pub fn register_by_name( + &mut self, + name: &'static CStr, + observer: Option, + action: Action, + ) { + match self.by_name.entry(name) { + Entry::Occupied(mut o) => { + o.get_mut().push((observer, action)); + } + Entry::Vacant(v) => { + v.insert(vec![(observer, action)]); + } + } + } + + /// Registers `action` to be invoked on `observer` for all notifications, + /// regardless of the notification's name or source. + pub fn register_universal(&mut self, observer: Option, action: Action) { + self.universal.push((observer, action)); + } + + /// Dispatches `notification` with registered actions. + pub fn dispatch(&mut self, notification: Notification) { + if let Some(observers) = self.by_name.get_mut(notification.name) { + for (observer, action) in observers { + let observer = observer.map(|x| x.ptr.as_ptr()).unwrap_or(ptr::null_mut()); + unsafe { + (action)(observer, ¬ification); + } + } + } + + if let Some(source) = notification.source { + if let Some(observers) = self.exact.get_mut(&(notification.name, source)) { + for (observer, action) in observers { + let observer = observer.map(|x| x.ptr.as_ptr()).unwrap_or(ptr::null_mut()); + unsafe { + (action)(observer, ¬ification); + } + } + } + if let Some(observers) = self.by_source.get_mut(&source) { + for (observer, action) in observers { + let observer = observer.map(|x| x.ptr.as_ptr()).unwrap_or(ptr::null_mut()); + unsafe { + (action)(observer, ¬ification); + } + } + } + } + + for (observer, action) in &mut self.universal { + let observer = observer.map(|x| x.ptr.as_ptr()).unwrap_or(ptr::null_mut()); + unsafe { + (action)(observer, ¬ification); + } + } + } + + /// Removes all notification subscriptions that would notify `observer` if they fired. + pub fn remove_observer(&mut self, observer: Sendable) { + self.exact.retain(|_, values| { + values.retain(|(o, _)| *o != Some(observer)); + !values.is_empty() + }); + self.by_name.retain(|_, values| { + values.retain(|(o, _)| *o != Some(observer)); + !values.is_empty() + }); + self.by_source.retain(|_, values| { + values.retain(|(o, _)| *o != Some(observer)); + !values.is_empty() + }); + self.universal.retain(|(o, _)| *o != Some(observer)); + } + + /// Clears all registered notification listeners and resets `self` to its + /// default state. + pub fn clear(&mut self) { + *self = Self::default(); + } +} + +pub mod ffi { + use super::{Action, Notification, NotificationCenter, Sendable}; + + use std::{ + ffi::{c_char, c_void, CStr}, + ptr::{self, NonNull}, + }; + + #[unsafe(no_mangle)] + pub unsafe extern "C" fn WMGetNotificationClientData( + notification: *mut Notification, + ) -> *mut c_void { + if notification.is_null() { + return ptr::null_mut(); + } + unsafe { + (*notification) + .client_data + .map(|x| x.ptr.as_ptr()) + .unwrap_or(ptr::null_mut()) + } + } + + #[unsafe(no_mangle)] + pub unsafe extern "C" fn WMGetNotificationObject( + notification: *mut Notification, + ) -> *mut c_void { + if notification.is_null() { + return ptr::null_mut(); + } + unsafe { + (*notification) + .source + .map(|x| x.ptr.as_ptr()) + .unwrap_or(ptr::null_mut()) + } + } + + #[unsafe(no_mangle)] + pub unsafe extern "C" fn WMGetNotificationName( + notification: *mut Notification, + ) -> *const c_char { + if notification.is_null() { + return ptr::null_mut(); + } + unsafe { (*notification).name.as_ptr() } + } + + #[unsafe(no_mangle)] + pub unsafe extern "C" fn WMAddNotificationObserver( + action: Option, + observer: *mut c_void, + name: *const c_char, + object: *mut c_void, + ) { + let Some(action) = action else { + return; + }; + let observer = NonNull::new(observer).map(|x| unsafe { Sendable::new(x) }); + let source = NonNull::new(object); + NotificationCenter::with_global_default(|c| { + if name.is_null() { + match source { + Some(source) => { + c.register_by_source(unsafe { Sendable::new(source) }, observer, action); + } + None => c.register_universal(observer, action), + } + } else { + let name = unsafe { CStr::from_ptr(name) }; + match source { + Some(source) => { + c.register_exact(name, unsafe { Sendable::new(source) }, observer, action); + } + None => c.register_by_name(name, observer, action), + } + } + }); + } + + #[unsafe(no_mangle)] + pub unsafe extern "C" fn WMRemoveNotificationObserver(observer: *mut c_void) { + let Some(observer) = NonNull::new(observer) else { + return; + }; + + NotificationCenter::with_global_default(|c| { + c.remove_observer(unsafe { Sendable::new(observer) }) + }); + } + + /// Posts a notification from `object` with the given `name` and `client_data`. + /// + /// ## Safety + /// + /// `name` must be a non-null string constant or some other pointer with a + /// static lifetime. + /// + /// `object` and `client_data` must be safe to send across threads (per the + /// contract of [`Sendable`]). + /// + /// ## Rust rewrite notes + /// + /// This originally took a heap-allocated `*mut Notification`, but now the + /// constructed `Notification` parameters are passed directly. + #[unsafe(no_mangle)] + pub unsafe extern "C" fn WMPostNotificationName( + name: *const c_char, + object: *mut c_void, + client_data: *mut c_void, + ) { + if name.is_null() { + return; + } + let name = unsafe { CStr::from_ptr(name) }; + let source = NonNull::new(object).map(|x| unsafe { Sendable::new(x) }); + let client_data = NonNull::new(client_data).map(|x| unsafe { Sendable::new(x) }); + NotificationCenter::with_global_default(|c| { + c.dispatch(Notification { + name, + source, + client_data, + }) + }); + } + + /// Resets all notifications for the global notification center. Used by + /// `WApplication` teardown code. This is a private, WINGs-internal API. + #[unsafe(no_mangle)] + pub unsafe extern "C" fn W_ClearNotificationCenter() { + NotificationCenter::with_global_default(|c| c.clear()); + } +} -- 2.39.5 From d8912c58e6d932e642cf92b9231fef231dfbdfc8 Mon Sep 17 00:00:00 2001 From: Stu Black Date: Mon, 8 Dec 2025 13:06:58 -0500 Subject: [PATCH 21/24] Fix typo in comment. --- wutil-rs/src/notification.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/wutil-rs/src/notification.rs b/wutil-rs/src/notification.rs index dc56c51e..1903ce03 100644 --- a/wutil-rs/src/notification.rs +++ b/wutil-rs/src/notification.rs @@ -49,7 +49,7 @@ pub type Action = unsafe extern "C" fn(observer: *mut c_void, notification: *con /// Wraps a type-erased pointer (which it does not own) and marks it as `Send`. /// -/// The `Send`-ability of the wrapped pointer must be guaranteed code that +/// The `Send`-ability of the wrapped pointer must be guaranteed by code that /// instantiates a `Sendable`. #[derive(Clone, Copy, Eq, PartialEq)] pub struct Sendable { -- 2.39.5 From d8057575ce35ee4b937b7bb58437e0171dfd959b Mon Sep 17 00:00:00 2001 From: Stu Black Date: Mon, 8 Dec 2025 13:14:12 -0500 Subject: [PATCH 22/24] Use a helper method for adding notification listeners. --- wutil-rs/src/notification.rs | 46 ++++++++++++++++-------------------- 1 file changed, 21 insertions(+), 25 deletions(-) diff --git a/wutil-rs/src/notification.rs b/wutil-rs/src/notification.rs index 1903ce03..d43d800d 100644 --- a/wutil-rs/src/notification.rs +++ b/wutil-rs/src/notification.rs @@ -1,11 +1,28 @@ use std::{ collections::{hash_map::Entry, HashMap}, ffi::{c_void, CStr}, - hash, + hash::{self, Hash}, ptr::{self, NonNull}, sync::{Mutex, OnceLock}, }; +// Helper function for adding the entry `(key, (observer, action))` to `map`. +fn register( + map: &mut HashMap, Action)>>, + key: K, + observer: Option, + action: Action, +) { + match map.entry(key) { + Entry::Occupied(mut o) => { + o.get_mut().push((observer, action)); + } + Entry::Vacant(v) => { + v.insert(vec![(observer, action)]); + } + } +} + /// Lightweight message from object to another. When a notification is sent, /// registered [`Action`]s are called. /// @@ -119,14 +136,7 @@ impl NotificationCenter { observer: Option, action: Action, ) { - match self.exact.entry((name, source)) { - Entry::Occupied(mut o) => { - o.get_mut().push((observer, action)); - } - Entry::Vacant(v) => { - v.insert(vec![(observer, action)]); - } - } + register(&mut self.exact, (name, source), observer, action); } /// Registers `action` to be invoked on `observer` when notifications are @@ -137,14 +147,7 @@ impl NotificationCenter { observer: Option, action: Action, ) { - match self.by_source.entry(source) { - Entry::Occupied(mut o) => { - o.get_mut().push((observer, action)); - } - Entry::Vacant(v) => { - v.insert(vec![(observer, action)]); - } - } + register(&mut self.by_source, source, observer, action); } /// Registers `action` to be invoked on `observer` for all notifications @@ -155,14 +158,7 @@ impl NotificationCenter { observer: Option, action: Action, ) { - match self.by_name.entry(name) { - Entry::Occupied(mut o) => { - o.get_mut().push((observer, action)); - } - Entry::Vacant(v) => { - v.insert(vec![(observer, action)]); - } - } + register(&mut self.by_name, name, observer, action); } /// Registers `action` to be invoked on `observer` for all notifications, -- 2.39.5 From 0893be1cea44386ab2148a584f5bc13d52115514 Mon Sep 17 00:00:00 2001 From: Stu Black Date: Mon, 8 Dec 2025 13:24:44 -0500 Subject: [PATCH 23/24] Use BTreeMap instead of HashMap for notification subscriptions. This allows us to initialize the global NotificationCenter singleton in a const context, which eliminates the need for OnceLock. --- wutil-rs/src/notification.rs | 38 ++++++++++++++++-------------------- 1 file changed, 17 insertions(+), 21 deletions(-) diff --git a/wutil-rs/src/notification.rs b/wutil-rs/src/notification.rs index d43d800d..45088a79 100644 --- a/wutil-rs/src/notification.rs +++ b/wutil-rs/src/notification.rs @@ -1,14 +1,13 @@ use std::{ - collections::{hash_map::Entry, HashMap}, + collections::{btree_map::Entry, BTreeMap}, ffi::{c_void, CStr}, - hash::{self, Hash}, ptr::{self, NonNull}, - sync::{Mutex, OnceLock}, + sync::Mutex, }; // Helper function for adding the entry `(key, (observer, action))` to `map`. -fn register( - map: &mut HashMap, Action)>>, +fn register( + map: &mut BTreeMap, Action)>>, key: K, observer: Option, action: Action, @@ -68,7 +67,7 @@ pub type Action = unsafe extern "C" fn(observer: *mut c_void, notification: *con /// /// The `Send`-ability of the wrapped pointer must be guaranteed by code that /// instantiates a `Sendable`. -#[derive(Clone, Copy, Eq, PartialEq)] +#[derive(Clone, Copy, Eq, Ord, PartialEq, PartialOrd)] pub struct Sendable { ptr: NonNull, } @@ -84,23 +83,16 @@ impl Sendable { } } -impl hash::Hash for Sendable { - fn hash(&self, h: &mut H) { - h.write_usize(self.ptr.as_ptr().addr()); - } -} - // Guaranteed by `Sendable::new`. unsafe impl Send for Sendable {} -#[derive(Default)] pub struct NotificationCenter { /// Notification subscriptions that match on name and source. - exact: HashMap<(&'static CStr, Sendable), Vec<(Option, Action)>>, + exact: BTreeMap<(&'static CStr, Sendable), Vec<(Option, Action)>>, /// Notification subscriptions that match on name. - by_name: HashMap<&'static CStr, Vec<(Option, Action)>>, + by_name: BTreeMap<&'static CStr, Vec<(Option, Action)>>, /// Notification subscriptions that match on source. - by_source: HashMap, Action)>>, + by_source: BTreeMap, Action)>>, /// Notification subscriptions that match all notifications. universal: Vec<(Option, Action)>, } @@ -111,8 +103,13 @@ unsafe impl Send for NotificationCenter {} impl NotificationCenter { /// Creates a new `NotificationCenter`. - pub fn new() -> Self { - Self::default() + pub const fn new() -> Self { + NotificationCenter { + exact: BTreeMap::new(), + by_name: BTreeMap::new(), + by_source: BTreeMap::new(), + universal: Vec::new(), + } } /// Provides access to the default, process-wide notification center. The @@ -120,9 +117,8 @@ impl NotificationCenter { /// mutex that is held while `f` is run, so panicking inside of `f` should /// be avoided. pub fn with_global_default(f: impl FnOnce(&mut Self) -> R) -> R { - static INSTANCE: OnceLock> = OnceLock::new(); + static INSTANCE: Mutex = Mutex::new(NotificationCenter::new()); f(&mut INSTANCE - .get_or_init(|| Mutex::new(NotificationCenter::new())) .try_lock() .unwrap()) } @@ -225,7 +221,7 @@ impl NotificationCenter { /// Clears all registered notification listeners and resets `self` to its /// default state. pub fn clear(&mut self) { - *self = Self::default(); + *self = Self::new(); } } -- 2.39.5 From d3dac752cc7193ebe041606c681e6fd8510225d6 Mon Sep 17 00:00:00 2001 From: Stu Black Date: Mon, 8 Dec 2025 13:31:05 -0500 Subject: [PATCH 24/24] Indent with tabs, as is standard for this file. --- WINGs/wtextfield.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/WINGs/wtextfield.c b/WINGs/wtextfield.c index b26ca5eb..5b7d1d1a 100644 --- a/WINGs/wtextfield.c +++ b/WINGs/wtextfield.c @@ -900,9 +900,9 @@ static void handleEvents(XEvent * event, void *data) paintTextField(tPtr); - if (tPtr->delegate && tPtr->delegate->didBeginEditing) { + if (tPtr->delegate && tPtr->delegate->didBeginEditing) { (*tPtr->delegate->didBeginEditing)(tPtr->delegate, 0); - } + } WMPostNotificationName(WMTextDidBeginEditingNotification, tPtr, NULL); tPtr->flags.notIllegalMovement = 0; @@ -1484,7 +1484,7 @@ static void handleTextFieldActionEvents(XEvent * event, void *data) text[n] = 0; WMInsertTextFieldText(tPtr, text, tPtr->cursorPosition); XFree(text); - if (tPtr->delegate && tPtr->delegate->didChange) { + if (tPtr->delegate && tPtr->delegate->didChange) { (*tPtr->delegate->didChange)(tPtr->delegate, WMInsertTextEvent); } WMPostNotificationName(WMTextDidChangeNotification, tPtr, (void *)WMInsertTextEvent); -- 2.39.5