XQF / xqf

XQF game server browser
http://xqf.github.io
GNU General Public License v2.0
37 stars 12 forks source link

Few minor fixes and improvements #217

Closed aufau closed 5 years ago

aufau commented 6 years ago

Hi, Few minor fixes and improvements I had laying around for over a year and forgot to pr them. On a bright side, I've been using them (with q3a, jk2 and jka) so they got some testing.

illwieckz commented 6 years ago

nice! that looks good! and thanks for the extensive testing :sweat_smile:

illwieckz commented 6 years ago

hmmmm, I get this message at startup:

free(): invalid next size (normal)

then it aborts, I will investigate more later

illwieckz commented 6 years ago

Note: it does not abort when running on gdb, that will not make debugging easier… :disappointed_relieved:

illwieckz commented 6 years ago

Hmmm, I don't know why gdb sees nothing but the nemiver frontend gave me that backtrace:

#0  __GI_raise(sig = 6, sig@entry = 6) at ../sysdeps/unix/sysv/linux/raise.c:51
#1  __GI_abort() at abort.c:79
#2  __libc_message(action = do_abort, action@entry = do_abort, fmt = 0x7ffff64d2b9a "%s\\n", fmt@entry = 0x7ffff64d2b9a "%s\\n") at ../sysdeps/posix/libc_fatal.c:181
#3  malloc_printerr(str = 0x7ffff64d4800 "free(): invalid next size (fast)", str@entry = 0x7ffff64d4800 "free(): invalid next size (fast)") at malloc.c:5350
#4  _int_free(have_lock = 0, p = 0x5555559661c0, av = 0x7ffff6707c40 <main_arena>) at malloc.c:4213
#5  __GI___libc_free(mem = 0x5555559661d0) at malloc.c:3124
#6  traverse_dir(startdir = 0x5555559604f0 "/home/illwieckz/.etqwcl", found_file = 0x5555555f3e21 <etqw_contains_file>, found_dir = 0x5555555f38e0 <quake_contains_dir>, data = 0x55555593d2a0) at /home/illwieckz/dev/xqf/src/q3maps.c:560
#7  find_etqw_maps(maphash = 0x55555593d2a0, startdir = 0x5555559604f0 "/home/illwieckz/.etqwcl") at /home/illwieckz/dev/xqf/src/q3maps.c:777
#8  etqw_init_maps(type = ETQW_SERVER) at /home/illwieckz/dev/xqf/src/game.c:3959
#9  scan_maps_for(type = ETQW_SERVER) at /home/illwieckz/dev/xqf/src/pref.c:4922
#10  prefs_load() at /home/illwieckz/dev/xqf/src/pref.c:4906
#11  main(argc = 1, argv = 0x7fffffffe218) at /home/illwieckz/dev/xqf/src/xqf.c:2696

it abort on this line on traverse_dir() function in q3maps.c file:

g_free(curdir);

when attempting to free curdir it has the /home/illwieckz/.etqwcl/base value (at 0x5555559661d0 in backtrace above).

aufau commented 6 years ago

I'll see into it, thanks

aufau commented 6 years ago

Judging by the error message, it looks to me like a double free() or some other malloc memory management error that probably happened before this specific g_free() in traverse_dir(). Are you sure it's introduced only in this branch? I can't seem to find the error by analysing the code, it would be easiest to debug it with valgrind or GCC's address sanitizer if I could reproduce the issue.

Overall there seem to be many memory management problems, noticed one leak just looking at traverse_dir() function. They mostly stem from glib's api and highly "optimized" usage of it in xqf making it hard to assert its validity I think. Again valgrind could be instrumental in finding these, although small leaks like this are rather harmless.

Looking at them now, my levelshot patch is not great, it assumes that levelshots are in levelshot/ directory which is not true for some id tech 3 games. I'll see into improving it.

aufau commented 6 years ago

Should be fine now.

illwieckz commented 5 years ago

sorry for the delay, it now looks ok, thank you very much ! :+1: