Skip to content

Commit

Permalink
xdo_search: fix improper memfrees, pre-compile regexes optimization
Browse files Browse the repository at this point in the history 
Original author: Peter Wu <[email protected]>
Modified by: Cycatz <[email protected]>

If any of compile_re statements fails, all uninitialized regex_t objects
are freed due to short-circuit evaluation, which causing segmentation faults.
Now they are initialized to zero with memset and besides that,
all regexes are now pre-compiled before iterating through all windows.

(as a side-effect of the optimization: return early when a regex fails
to compile).

Other changes: constantify regex_t* where applicable.
  • Loading branch information
@Cycatz
Cycatz committed Oct 17, 2022
1 parent df94eb6 commit d4d7fee
Showing 1 changed file with 70 additions and 50 deletions.
120 changes: 70 additions & 50 deletions xdo_search.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,57 +12,77 @@
#include <X11/Xutil.h>
#include <X11/extensions/XTest.h>
#include "xdo.h"

#include <string.h>

/* Pre-compiled regular expressions for window matching */
typedef struct {
regex_t title;
regex_t class;
regex_t classname;
regex_t name;
regex_t role;
} xdo_regex_t;

static void _free_regexes(xdo_regex_t *regexes);
static int _compile_regexes(const xdo_search_t *search, xdo_regex_t *regexes);
static int compile_re(const char *pattern, regex_t *re);
static int check_window_match(const xdo_t *xdo, Window wid, const xdo_search_t *search);
static int _xdo_match_window_class(const xdo_t *xdo, Window window, regex_t *re);
static int _xdo_match_window_classname(const xdo_t *xdo, Window window, regex_t *re);
static int _xdo_match_window_role(const xdo_t *xdo, Window window, regex_t *re);
static int _xdo_match_window_name(const xdo_t *xdo, Window window, regex_t *re);
static int _xdo_match_window_title(const xdo_t *xdo, Window window, regex_t *re);
static int check_window_match(const xdo_t *xdo, Window wid,
const xdo_search_t *search,
const xdo_regex_t *regexes);
static int _xdo_match_window_class(const xdo_t *xdo, Window window, const regex_t *re);
static int _xdo_match_window_classname(const xdo_t *xdo, Window window, const regex_t *re);
static int _xdo_match_window_role(const xdo_t *xdo, Window window, const regex_t *re);
static int _xdo_match_window_name(const xdo_t *xdo, Window window, const regex_t *re);
static int _xdo_match_window_title(const xdo_t *xdo, Window window, const regex_t *re);
static int _xdo_match_window_pid(const xdo_t *xdo, Window window, int pid);
static int _xdo_is_window_visible(const xdo_t *xdo, Window wid);
static void find_matching_windows(const xdo_t *xdo, Window window,
static void find_matching_windows(const xdo_t *xdo, Window window,
const xdo_search_t *search,
const xdo_regex_t *regexes,
Window **windowlist_ret,
unsigned int *nwindows_ret,
unsigned int *windowlist_size,
int current_depth);

int xdo_search_windows(const xdo_t *xdo, const xdo_search_t *search,
Window **windowlist_ret, unsigned int *nwindows_ret) {
xdo_regex_t regexes;
int i = 0;

unsigned int windowlist_size = 100;
*nwindows_ret = 0;
*windowlist_ret = calloc(sizeof(Window), windowlist_size);

if (!_compile_regexes(search, &regexes)) {
return XDO_ERROR;
}

/* TODO(sissel): Support multiple screens */
if (search->searchmask & SEARCH_SCREEN) {
Window root = RootWindow(xdo->xdpy, search->screen);
if (check_window_match(xdo, root, search)) {
if (check_window_match(xdo, root, search, &regexes)) {
(*windowlist_ret)[*nwindows_ret] = root;
(*nwindows_ret)++;
/* Don't have to check for size bounds here because
* we start with array size 100 */
}

/* Start with depth=1 since we already covered the root windows */
find_matching_windows(xdo, root, search, windowlist_ret, nwindows_ret,
&windowlist_size, 1);
find_matching_windows(xdo, root, search, &regexes, windowlist_ret,
nwindows_ret, &windowlist_size, 1);
} else {
const int screencount = ScreenCount(xdo->xdpy);
for (i = 0; i < screencount; i++) {
Window root = RootWindow(xdo->xdpy, i);
if (check_window_match(xdo, root, search)) {
if (check_window_match(xdo, root, search, &regexes)) {
(*windowlist_ret)[*nwindows_ret] = root;
(*nwindows_ret)++;
/* Don't have to check for size bounds here because
* we start with array size 100 */
}

/* Start with depth=1 since we already covered the root windows */
find_matching_windows(xdo, root, search, windowlist_ret,
find_matching_windows(xdo, root, search, &regexes, windowlist_ret,
nwindows_ret, &windowlist_size, 1);
}
}
Expand All @@ -77,16 +97,17 @@ int xdo_search_windows(const xdo_t *xdo, const xdo_search_t *search,
//printf("classname: %s\n", search->winclassname);
//printf("//Search\n");

_free_regexes(&regexes);
return XDO_SUCCESS;
} /* int xdo_search_windows */

static int _xdo_match_window_title(const xdo_t *xdo, Window window, regex_t *re) {
static int _xdo_match_window_title(const xdo_t *xdo, Window window, const regex_t *re) {
fprintf(stderr, "This function (match window by title) is deprecated."
" You want probably want to match by the window name.\n");
return _xdo_match_window_name(xdo, window, re);
} /* int _xdo_match_window_title */

static int _xdo_match_window_name(const xdo_t *xdo, Window window, regex_t *re) {
static int _xdo_match_window_name(const xdo_t *xdo, Window window, const regex_t *re) {
/* historically in xdo, 'match_name' matched the classhint 'name' which we
* match in _xdo_match_window_classname. But really, most of the time 'name'
* refers to the window manager name for the window, which is displayed in
Expand Down Expand Up @@ -121,7 +142,7 @@ static int _xdo_match_window_name(const xdo_t *xdo, Window window, regex_t *re)
return False;
} /* int _xdo_match_window_name */

static int _xdo_match_window_class(const xdo_t *xdo, Window window, regex_t *re) {
static int _xdo_match_window_class(const xdo_t *xdo, Window window, const regex_t *re) {
XWindowAttributes attr;
XClassHint classhint;
XGetWindowAttributes(xdo->xdpy, window, &attr);
Expand All @@ -144,7 +165,7 @@ static int _xdo_match_window_class(const xdo_t *xdo, Window window, regex_t *re)
return False;
} /* int _xdo_match_window_class */

static int _xdo_match_window_classname(const xdo_t *xdo, Window window, regex_t *re) {
static int _xdo_match_window_classname(const xdo_t *xdo, Window window, const regex_t *re) {
XWindowAttributes attr;
XClassHint classhint;
XGetWindowAttributes(xdo->xdpy, window, &attr);
Expand All @@ -166,7 +187,7 @@ static int _xdo_match_window_classname(const xdo_t *xdo, Window window, regex_t
return False;
} /* int _xdo_match_window_classname */

static int _xdo_match_window_role(const xdo_t *xdo, Window window, regex_t *re) {
static int _xdo_match_window_role(const xdo_t *xdo, Window window, const regex_t *re) {
int status;
int ret = False;
int i;
Expand Down Expand Up @@ -210,8 +231,7 @@ static int _xdo_match_window_pid(const xdo_t *xdo, Window window, const int pid)
static int compile_re(const char *pattern, regex_t *re) {
int ret;
if (pattern == NULL) {
regcomp(re, "^$", REG_EXTENDED | REG_ICASE);
return True;
pattern = "^$";
}

ret = regcomp(re, pattern, REG_EXTENDED | REG_ICASE);
Expand All @@ -222,6 +242,31 @@ static int compile_re(const char *pattern, regex_t *re) {
return True;
} /* int compile_re */

static void _free_regexes(xdo_regex_t *regexes) {
regfree(&regexes->title);
regfree(&regexes->class);
regfree(&regexes->classname);
regfree(&regexes->name);
regfree(&regexes->role);
memset(regexes, 0, sizeof(*regexes));
} /* void _free_regexes */

/* returns False if one of the regexes failed to compile. If True, then the
* resulting regexes must be freed */
static int _compile_regexes(const xdo_search_t *search, xdo_regex_t *regexes) {
memset(regexes, 0, sizeof(*regexes));

if (!compile_re(search->title, &regexes->title)
|| !compile_re(search->winclass, &regexes->class)
|| !compile_re(search->winclassname, &regexes->classname)
|| !compile_re(search->winname, &regexes->name)
|| !compile_re(search->winrole, &regexes->role)) {
_free_regexes(regexes);
return False;
}
return True;
} /* int _compile_regexes */

static int _xdo_is_window_visible(const xdo_t *xdo, Window wid) {
XWindowAttributes wattr;
XGetWindowAttributes(xdo->xdpy, wid, &wattr);
Expand All @@ -232,29 +277,8 @@ static int _xdo_is_window_visible(const xdo_t *xdo, Window wid) {
} /* int _xdo_is_window_visible */

static int check_window_match(const xdo_t *xdo, Window wid,
const xdo_search_t *search) {
regex_t title_re;
regex_t class_re;
regex_t classname_re;
regex_t name_re;
regex_t role_re;


if (!compile_re(search->title, &title_re) \
|| !compile_re(search->winclass, &class_re) \
|| !compile_re(search->winclassname, &classname_re) \
|| !compile_re(search->winrole, &role_re) \
|| !compile_re(search->winname, &name_re)) {

regfree(&title_re);
regfree(&class_re);
regfree(&classname_re);
regfree(&name_re);
regfree(&role_re);

return False;
}

const xdo_search_t *search,
const xdo_regex_t *regexes) {
/* Set this to 1 for dev debugging */
static const int debug = 0;

Expand Down Expand Up @@ -331,11 +355,6 @@ static int check_window_match(const xdo_t *xdo, Window wid,
}
} while (0);

regfree(&title_re);
regfree(&class_re);
regfree(&classname_re);
regfree(&name_re);
regfree(&role_re);

if (debug) {
fprintf(stderr, "win: %ld, pid:%d, title:%d, name:%d, class:%d, visible:%d\n",
Expand Down Expand Up @@ -387,6 +406,7 @@ static int ignore_badwindow(Display *dpy, XErrorEvent *xerr) {

static void find_matching_windows(const xdo_t *xdo, Window window,
const xdo_search_t *search,
const xdo_regex_t *regexes,
Window **windowlist_ret,
unsigned int *nwindows_ret,
unsigned int *windowlist_size,
Expand Down Expand Up @@ -430,7 +450,7 @@ static void find_matching_windows(const xdo_t *xdo, Window window,
/* Breadth first, check all children for matches */
for (i = 0; i < nchildren; i++) {
Window child = children[i];
if (!check_window_match(xdo, child, search))
if (!check_window_match(xdo, child, search, regexes))
continue;

(*windowlist_ret)[*nwindows_ret] = child;
Expand All @@ -452,7 +472,7 @@ static void find_matching_windows(const xdo_t *xdo, Window window,
/* Now check children-children */
if (search->max_depth == -1 || (current_depth + 1) <= search->max_depth) {
for (i = 0; i < nchildren; i++) {
find_matching_windows(xdo, children[i], search, windowlist_ret,
find_matching_windows(xdo, children[i], search, regexes, windowlist_ret,
nwindows_ret, windowlist_size,
current_depth + 1);
}
Expand Down

0 comments on commit d4d7fee

Please sign in to comment.