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