DRY
In create_win() there are two calls to XCreateWindow(), each having a long, long list of parameters.
Turns out, only one of the parameters is different.
While each branch of the if/else performs different preparatory work, please find a way to use a stack variable to distinguish root from *parent. (It feels like the datatype Window hides its nature; being a pointer. Hidden 'splats' are an anathema.) This could eliminate one instance of a monster function call.
Trivial
Many of the 2D token names are based on Height and Width, but then others use x and y (which refer to "width" and "height" respectively.) Note how the 'sequence' is subtly swapped.
Please stick to one convention. Perhaps int h = SOME_HEIGHT, w = SOME_WIDTH;.
Reduce the burden on the reader who is trying to determine which way is up in unfamiliar code.
Preprocessor macros or enums
The preprocessor is a wonderful "editing" tool, allowing tokens to be replaced by constants (and much more) before the compiler even sees the code.
The downside is that those tokens that can be read in source code are not available when debugging.
Suggestion:
enum { POSX = 500, POSY = 500, WIDTH = 750, HEIGHT = 500, BORDER = 5, LINE = 4 }; These compile time token names are available within a debug build.
More (kinda) trivial
When working with code written/maintained by others, it's probably best to adapt to their conventions (as much as possible.)
// static Window create_win( // snake case static Window createWin( // camel case is, at least, half way to their standard Worrisome
create_gc() uses an uninitialised struct XGCValues xgcv; defined in its current stack frame. The code goes on to explicitly set some struct members. Are ALL members assigned a definite value? The nervous reader is compelled to seek out documentation hoping to quell their uncertainty.
Suggest:
XGCValues xgcv = {0}; to ensure that ALL members will always be set consistently. Fluctuating values of uninitialised stack variables are a major pain to diagnose. (Sometimes the program works; sometimes it doesn't.) Generally, the reader should be able to see all variables being assigned definite values.
The same sort of trepidation arises from
color_buttons[i].color->pixel |= 0xff << 24; This line mysteriously initialises one member of a struct whose complete definition (and use) requires a bit of research. malloc() is not calloc(), so there's every possibility that uninitialised values may be different between compilations. What appeared to work for the past month suddenly exhibits bizarre behaviour.
Don't allow variables to wander off uninitialised!
Finally, why create this alias?
typedef XftColor Clr; There are so many variations of "color" (abbreviated, plural, w/ suffix) that the code becomes difficult to track. As noted by @Chux, the only place Clr is used is where it should not have been used. Try to simplify, not to make things more opaque.
Avoid extra variables
There's a very small gain to be had by eliminating one superfluous variable:
int flag = 0; // Say bye-bye, flag! for (int i = 0; i < NUM_COLORS; i++) { if (cur_win == color_buttons[i].win) { XSetForeground(dpy, ... flag = 1; break; } } if (!flag) XDrawPoint(dpy, ... And, without flag:
int i = 0; while( i < NUM_COLORS && cur_win != color_buttons[i].win ) i++; // scanning through array if ( i < NUM_COLORS ) XSetForeground(dpy, ... else XDrawPoint(dpy, ... It's fewer lines, and one trivial stack variable removed.
In this instance, the reader is trying to keep track of these (unfamiliar?) library objects and functions, the sequence of operations, the coder's approach to solving the project, and a bevy of transient variables.
As long as clarity (and correctness!) is not sacrificed, strive to maximise the utility of fewer resources.