[PATCH] Refactor xsetcolorname() and xloadcols()

From: FRIGN <dev_AT_frign.de>
Date: Mon, 26 May 2014 16:32:34 +0200

I mainly improved the slightly off algorithm used to load colours in the 256-colour-space and
removed unnecessary local values (r,g,b,colour).
"colour" is not necessary as a punchbag for XftColorAlloc[Value,Name], as they don't mess with
the result-adress until they are absolutely sure everything worked out[0].

Being at it, I changed the error-returns for AllocValue to dies (just like in xloadcols()), as
a failure is most likely an OOM-situation you better catch early.
In case of an invalid name everything stays the same.

Now, due to duplicate code, I simplified xloadcols() to use xsetcolorname(i, NULL).

[0]: http://www.opensource.apple.com/source/X11libs/X11libs-40/libXft/libXft-2.1.13/src/xftcolor.c

---
 st.c | 75 +++++++++++++++++++++++++-------------------------------------------
 1 file changed, 27 insertions(+), 48 deletions(-)
diff --git a/st.c b/st.c
index 6424b54..b1efeaf 100644
--- a/st.c
+++ b/st.c
_AT_@ -2716,71 +2716,50 @@ sixd_to_16bit(int x) {
 void
 xloadcols(void) {
 	int i;
-	XRenderColor color = { .alpha = 0xffff };
 	static bool loaded;
 	Colour *cp;
 
-	if(loaded) {
-		for (cp = dc.col; cp < dc.col + LEN(dc.col); ++cp)
+	if(loaded)
+		for(cp = dc.col; cp < dc.col + LEN(dc.col); ++cp)
 			XftColorFree(xw.dpy, xw.vis, xw.cmap, cp);
-	}
 
-	/* load colours [0-15] and [256-LEN(colorname)] (config.h) */
-	for(i = 0; i < LEN(colorname); i++) {
-		if(!colorname[i])
-			continue;
-		if(!XftColorAllocName(xw.dpy, xw.vis, xw.cmap, colorname[i], &dc.col[i])) {
-			die("Could not allocate color '%s'\n", colorname[i]);
-		}
-	}
-
-	/* load colours [16-231] ; same colours as xterm */
-	for(i = 16; i < 6*6*6+16; i++) {
-		color.red   = sixd_to_16bit( ((i-16)/36)%6 );
-		color.green = sixd_to_16bit( ((i-16)/6) %6 );
-		color.blue  = sixd_to_16bit( ((i-16)/1) %6 );
-		if(!XftColorAllocValue(xw.dpy, xw.vis, xw.cmap, &color, &dc.col[i]))
-			die("Could not allocate color %d\n", i);
-	}
-
-	/* load colours [232-255] ; grayscale */
-	for(; i < 256; i++) {
-		color.red = color.green = color.blue = 0x0808 + 0x0a0a * (i-(6*6*6+16));
-		if(!XftColorAllocValue(xw.dpy, xw.vis, xw.cmap, &color, &dc.col[i]))
-			die("Could not allocate color %d\n", i);
-	}
+	for(i = 0; i < LEN(colorname); i++)
+		if(!xsetcolorname(i, NULL))
+			fprintf(stderr, "erresc: invalid color %s\n", colorname[i]);
 	loaded = true;
 }
 
-int
-xsetcolorname(int x, const char *name) {
+inline int
+xsetcolorname(const int x, const char *name) {
 	XRenderColor color = { .alpha = 0xffff };
-	Colour colour;
+
 	if(!BETWEEN(x, 0, LEN(colorname)))
-		return -1;
+		return -1; /* invalid index */
+
 	if(!name) {
-		if(BETWEEN(x, 16, 16 + 215)) {
-			int r = (x - 16) / 36, g = ((x - 16) % 36) / 6, b = (x - 16) % 6;
-			color.red = sixd_to_16bit(r);
-			color.green = sixd_to_16bit(g);
-			color.blue = sixd_to_16bit(b);
-			if(!XftColorAllocValue(xw.dpy, xw.vis, xw.cmap, &color, &colour))
-				return 0; /* something went wrong */
-			dc.col[x] = colour;
+		if(BETWEEN(x, 16, 6*6*6+16-1)) {
+			/* 256 colour [16-231] ; same colours as xterm */
+			color.red   = sixd_to_16bit( ((x-16)/36)%6 );
+			color.green = sixd_to_16bit( ((x-16)/6) %6 );
+			color.blue  = sixd_to_16bit( ((x-16)/1) %6 );
+			if (!XftColorAllocValue(xw.dpy, xw.vis, xw.cmap, &color, &dc.col[x]))
+				die("Could not allocate color %d\n", x);
 			return 1;
-		} else if(BETWEEN(x, 16 + 216, 255)) {
-			color.red = color.green = color.blue = 0x0808 + 0x0a0a * (x - (16 + 216));
-			if(!XftColorAllocValue(xw.dpy, xw.vis, xw.cmap, &color, &colour))
-				return 0; /* something went wrong */
-			dc.col[x] = colour;
+		} else if(BETWEEN(x, 6*6*6+16, 255)) {
+			/* grayscale [232-255] */
+			color.red = color.green = color.blue = 0x0808 + 0x0a0a * (x-(6*6*6+16));
+			if (!XftColorAllocValue(xw.dpy, xw.vis, xw.cmap, &color, &dc.col[x]))
+				die("Could not allocate color %d\n", x);
 			return 1;
 		} else {
+			/* system colours [0-15, 256-LEN(colorname)] */
+			if (!colorname[x])
+				return -1;
 			name = colorname[x];
 		}
 	}
-	if(!XftColorAllocName(xw.dpy, xw.vis, xw.cmap, name, &colour))
-		return 0;
-	dc.col[x] = colour;
+	if(!XftColorAllocName(xw.dpy, xw.vis, xw.cmap, name, &dc.col[x]))
+		return 0; /* invalid name */
 	return 1;
 }
 
-- 
1.8.3.2
--Multipart=_Mon__26_May_2014_16_36_42_+0200_mym8VRxE4WGadp6Y--

Received on Mon Sep 17 2001 - 00:00:00 CEST