code cleaning - stage 2
Mihai Rusu
dizzy at roedu.net
Mon May 10 18:07:51 EDT 2004
Hi
For quite some time I use a super debugging tool to detect and fix memory
utilization bugs. Its called valgrind, more information can be found at
http://valgrind.kde.org.
I would like to disucss some possible problems found by valgrind bellow.
Information is based on icculus.org latest CVS:
==12654== Conditional jump or move depends on uninitialised value(s)
==12654== at 0x814EBBE: ds_play(int, int, int, int, int, int, int, bool) (ds.cpp:2593)
==12654== by 0x814CAF0: snd_play_looping(game_snd*, float, int, int, float, int, int) (sound.cpp:1080)
==12654== by 0x80C968A: main_hall_start_ambient() (mainhallmenu.cpp:1859)
==12654== by 0x80D41E8: player_select_init() (playermenu.cpp:449)
==12654== by 0x805280C: game_enter_state(int, int) (freespace.cpp:6459)
==12654== by 0x8077A43: gameseq_set_state(int, int) (gamesequence.cpp:497)
==12654== by 0x8051B56: game_process_event(int, int) (freespace.cpp:5635)
==12654== by 0x8077C8C: gameseq_process_events() (gamesequence.cpp:610)
==12654== by 0x80536F3: WinMainSub(int, int, char*, int) (freespace.cpp:7203)
==12654== by 0x805373A: WinMain(int, int, char*, int) (freespace.cpp:7241)
Seems to me that Channels[i].is_voice_msg doesnt get set/initilized
properly for every channel < MAX_CHANNELS. In what function should this
happen ?
==12654== Conditional jump or move depends on uninitialised value(s)
==12654== at 0x814F30C: ds_do_frame() (ds.cpp:3759)
==12654== by 0x814D36A: snd_do_frame() (sound.cpp:1575)
==12654== by 0x8052843: game_do_state_common(int, int) (freespace.cpp:6485)
==12654== by 0x80528AC: game_do_state(int) (freespace.cpp:6525)
==12654== by 0x8077CC7: gameseq_process_events() (gamesequence.cpp:620)
==12654== by 0x80536F3: WinMainSub(int, int, char*, int) (freespace.cpp:7203)
==12654== by 0x805373A: WinMain(int, int, char*, int) (freespace.cpp:7241)
==12654== by 0x8054D50: main (unixmain.cpp:40)
==12654== by 0x404C3C63: __libc_start_main (in /lib/libc-2.3.2.so)
==12654== by 0x804B990: (within /home/dizzy/work/cvs/freespace2/freespace2)
Same problem here.
==12654== Use of uninitialised value of size 4
==12654== at 0x8080EB7: opengl_create_texture_sub(int, int, unsigned short*, int, int, int, int, int, int, int, int, tcache_slot_opengl*, int, int) (gropengl.cpp:2152)
==12654== by 0x808145D: opengl_create_texture(int, int, tcache_slot_opengl*, int) (gropengl.cpp:2314)
==12654== by 0x8081907: gr_opengl_tcache_set(int, int, float*, float*, int, int, int, int) (gropengl.cpp:2468)
==12654== by 0x807DC00: gr_opengl_aabitmap_ex_internal(int, int, int, int, int, int) (gropengl.cpp:807)
==12654== by 0x807E423: gr_opengl_string(int, int, char*) (gropengl.cpp:1026)
==12654== by 0x81BB904: UI_WINDOW::draw_one_xstr(UI_XSTR*, int) (window.cpp:759)
==12654== by 0x81BB949: UI_WINDOW::draw_xstrs() (window.cpp:774)
==12654== by 0x81BB07B: UI_WINDOW::draw() (window.cpp:379)
==12654== by 0x80D49F7: player_select_do() (playermenu.cpp:693)
==12654== by 0x8052C96: game_do_state(int) (freespace.cpp:6737)
Seems that xlat[bmp_data[i*bmap_w+j]] wasnt set/initilized before reading
it.
==12654== Source and destination overlap in strncpy(0x872e789, 0x872e789, 32)
==12654== at 0x40022156: strncpy (in /usr/lib/valgrind/vgskin_memcheck.so)
==12654== by 0x8158D8C: player_set_squad_bitmap(player*, char*) (managepilot.cpp:1377)
==12654== by 0x815707D: read_pilot_file(char*, int, player*) (managepilot.cpp:621)
==12654== by 0x80D4B32: player_select_close() (playermenu.cpp:774)
==12654== by 0x8052105: game_leave_state(int, int) (freespace.cpp:6015)
==12654== by 0x8077A15: gameseq_set_state(int, int) (gamesequence.cpp:493)
==12654== by 0x805150B: game_process_event(int, int) (freespace.cpp:5345)
==12654== by 0x8077C8C: gameseq_process_events() (gamesequence.cpp:610)
==12654== by 0x80536F3: WinMainSub(int, int, char*, int) (freespace.cpp:7203)
==12654== by 0x805373A: WinMain(int, int, char*, int) (freespace.cpp:7241)
This while probably harmless on many libc on x86 it can prove dangerous on
other architectures/in time because strcpy should not ever overlap. I dont
have the knowledge to either decide if change this to memmove() or the bug
comes from the calling codes.
This "bugs" I found untile the "select player" window. Ill dig some more
when I have more time if there is any interest.
--
Mihai RUSU Email: dizzy at roedu.net
GPG : http://dizzy.roedu.net/dizzy-gpg.txt WWW: http://dizzy.roedu.net
"Linux is obsolete" -- AST
More information about the freespace2
mailing list