cococry / ragnar

Minimal, flexible & user-friendly X tiling window manager
https://ragnarwm.org
GNU General Public License v3.0
1.03k stars 21 forks source link

Ragnar crash if no configuration is provided #58

Open Sigmanificient opened 1 month ago

Sigmanificient commented 1 month ago

Describe the bug If ~/.config/ragnarwm/ragnar.cfg is not present, ragnar will crash will trying to initialize.

Expected behavior Ragnar should start with a default configuration (maybe the one provided in the repository)

Valgrind dump

[nix-shell:~/ragnar]$ DISPLAY=:1 valgrind bin/ragnar 
==837456== Memcheck, a memory error detector
==837456== Copyright (C) 2002-2024, and GNU GPL'd, by Julian Seward et al.
==837456== Using Valgrind-3.23.0 and LibVEX; rerun with -h for copyright info
==837456== Command: bin/ragnar
==837456== 
==837456== Conditional jump or move depends on uninitialised value(s)
==837456==    at 0x407F8C: logmsg (ragnar.c:3469)
==837456==    by 0x405F68: initconfig (config.c:549)
==837456==    by 0x40D4D4: setup (ragnar.c:117)
==837456==    by 0x404B05: main (ragnar.c:3599)
==837456== 
==837456== Conditional jump or move depends on uninitialised value(s)
==837456==    at 0x4081D4: terminate (ragnar.c:261)
==837456==    by 0x405F7A: initconfig (config.c:553)
==837456==    by 0x40D4D4: setup (ragnar.c:117)
==837456==    by 0x404B05: main (ragnar.c:3599)
==837456== 
==837456== Conditional jump or move depends on uninitialised value(s)
==837456==    at 0x408201: terminate (ragnar.c:271)
==837456==    by 0x405F7A: initconfig (config.c:553)
==837456==    by 0x40D4D4: setup (ragnar.c:117)
==837456==    by 0x404B05: main (ragnar.c:3599)
==837456== 
==837456== Use of uninitialised value of size 8
==837456==    at 0x48E9C79: XCloseDisplay (in /nix/store/qp943rnqymvqx3w57xrvgiamsr57hhf2-libX11-1.8.10/lib/libX11.so.6.4.0)
==837456==    by 0x408224: terminate (ragnar.c:278)
==837456==    by 0x405F7A: initconfig (config.c:553)
==837456==    by 0x40D4D4: setup (ragnar.c:117)
==837456==    by 0x404B05: main (ragnar.c:3599)
==837456== 
==837456== Invalid read of size 8
==837456==    at 0x48E9C79: XCloseDisplay (in /nix/store/qp943rnqymvqx3w57xrvgiamsr57hhf2-libX11-1.8.10/lib/libX11.so.6.4.0)
==837456==    by 0x408224: terminate (ragnar.c:278)
==837456==    by 0x405F7A: initconfig (config.c:553)
==837456==    by 0x40D4D4: setup (ragnar.c:117)
==837456==    by 0x404B05: main (ragnar.c:3599)
==837456==  Address 0xf8 is not stack'd, malloc'd or (recently) free'd
==837456== 
==837456== 
==837456== Process terminating with default action of signal 11 (SIGSEGV): dumping core
==837456==  Access not within mapped region at address 0xF8
==837456==    at 0x48E9C79: XCloseDisplay (in /nix/store/qp943rnqymvqx3w57xrvgiamsr57hhf2-libX11-1.8.10/lib/libX11.so.6.4.0)
==837456==    by 0x408224: terminate (ragnar.c:278)
==837456==    by 0x405F7A: initconfig (config.c:553)
==837456==    by 0x40D4D4: setup (ragnar.c:117)
==837456==    by 0x404B05: main (ragnar.c:3599)
==837456==  If you believe this happened as a result of a stack
==837456==  overflow in your program's main thread (unlikely but
==837456==  possible), you can try to increase the size of the
==837456==  main thread stack using the --main-stacksize= flag.
==837456==  The main thread stack size used in this run was 8388608.
==837456== 
==837456== HEAP SUMMARY:
==837456==     in use at exit: 2,871 bytes in 50 blocks
==837456==   total heap usage: 52 allocs, 2 frees, 3,407 bytes allocated
==837456== 
==837456== LEAK SUMMARY:
==837456==    definitely lost: 0 bytes in 0 blocks
==837456==    indirectly lost: 0 bytes in 0 blocks
==837456==      possibly lost: 160 bytes in 2 blocks
==837456==    still reachable: 2,711 bytes in 48 blocks
==837456==         suppressed: 0 bytes in 0 blocks
==837456== Rerun with --leak-check=full to see details of leaked memory
==837456== 
==837456== Use --track-origins=yes to see where uninitialised values come from
==837456== For lists of detected and suppressed errors, rerun with: -s
==837456== ERROR SUMMARY: 5 errors from 5 contexts (suppressed: 0 from 0)
Segmentation fault (core dumped)

The segmentation fault can be fixed with this simple patch (initializing wm_state cleanly, checking before closing the display):

diff --git a/src/ragnar.c b/src/ragnar.c
index 7dde210..3af70ce 100644
--- a/src/ragnar.c
+++ b/src/ragnar.c
@@ -275,7 +275,8 @@ terminate(state_t* s, int32_t exitcode) {
     }
   }

-  XCloseDisplay(s->dsp);
+  if (s->dsp != NULL)
+      XCloseDisplay(s->dsp);
   // Give up the X connection
   xcb_disconnect(s->con);

@@ -3594,7 +3595,7 @@ cmdoutput(const char* cmd) {

 int
 main(void) {
-  state_t* wm_state = malloc(sizeof(state_t));
+  state_t* wm_state = calloc(1, sizeof(state_t));
   // Setup the window manager
   setup(wm_state);
   // Manage all windows on the display