jumper149/blugon

[scg] XCloseDisplay could be called before normal exit

Closed this issue · 3 comments

from man 3 XCloseDisplay

The XCloseDisplay function closes the connection to the X server for
the display specified in the Display structure ...
...
Before exiting, you should call XCloseDisplay explicitly so that any pending
errors are reported as XCloseDisplay performs a final XSync operation.

an XRRFreeScreenResources might be nice too, so something like the following:

diff --git a/backends/scg/scg.c b/backends/scg/scg.c
index 57951dc..f152a80 100644
--- a/backends/scg/scg.c
+++ b/backends/scg/scg.c
@@ -41,14 +41,16 @@ int main(int argc, char **argv) {
 			crtc_gamma->green[i] = g * gamma_g;
 			crtc_gamma->blue[i]  = g * gamma_b;
 		}
 		XRRSetCrtcGamma(dpy, crtcxid, crtc_gamma);
 
 		XFree(crtc_gamma);
 	}
+	XRRFreeScreenResources(res);
+	XCloseDisplay(dpy);
 	return 0;
 }
 
 /*
  * This program is based on 'sct'.
  * Source: https://https.www.google.com.tedunangst.com/flak/post/sct-set-color-temperature
  *         https://https.www.google.com.tedunangst.com/flak/files/sct.c

XCloseDisplay() seems really fitting here!

I'm not too sure about XRRFreeResources() though. The system will free all the memory of the program right after exiting, so freeing individual resources beforehand is actually a little slower. It still looks cleaner to me with calling XRRFreeResources().

Thank you for your help! :)

Does anyone else have an opinion on freeing right before exiting?

I'm unsure if the call to XCloseDisplay would depend in any way on having called XRRFreeResources beforehand, I'll test...

apparently not, but I didn't try to check if any errors were generated by XCloseDisplay using an error handler, however I don't think any other errors would be generated than BadGC A value for a GContext argument does not name a defined GContext . (because the man page says XCloseDisplay() can generate a BadGC error.), but then again I've zero experience with programming anything for X, so I wouldn't know.

EDIT: I've tried with an error handler too, but didn't see any errors.

I'm gonna merge it as you proposed for now. With some tests I didn't encounter any problems with it.