Other annoying bugs (Read all)
< Newer Topic
:: Older Topic >
#21 Aug 28, 2009 3:42 pm
Geomancer
GroupAdministrators
Posts1,946
JoinedJul 26, 2005
Good, now if we figure out some of these leaks we might be good to go lol.
On that lightsaber bug you posted about I would have just changed it to use the damage function also or have it update_position etc... But everyone seemed to be against that so I'll let someone else decide on the best way to deal with it.
On that lightsaber bug you posted about I would have just changed it to use the damage function also or have it update_position etc... But everyone seemed to be against that so I'll let someone else decide on the best way to deal with it.
#22 Aug 28, 2009 6:12 pm
Magician
GroupMembers
Posts239
JoinedJun 13, 2008
Mem leaks are going to be a pain. Not my cup of tea - know little about valgrind.
As to the lightsaber - might not be a bad idea to look over all the other force skills see if there's anything else like that. I don't think there was, but that's been a while ago.
ayuri
As to the lightsaber - might not be a bad idea to look over all the other force skills see if there's anything else like that. I don't think there was, but that's been a while ago.
ayuri
#23 Aug 28, 2009 7:21 pm
Geomancer
GroupAdministrators
Posts1,946
JoinedJul 26, 2005
Well on this leak I already found it, but I decided it should be fine but I don't know all that much about how all this base works so here is the leak.
When you buy a ship it creates a room for the cockpit. This room is actually where you get memory left even if you use sellship. Since the room is still left in memory. I'm not sure if the room should be removed or not etc.... I found it by tossing in checks in STRALLOC and STRFREE to tell me what was getting allocated and freed and i knoticed that only 2 things between buyship and sellship weren't getting removed. One is a "" and the other was like room flags being put into room->flags. And so then I checked to see and found the room still stays there after the ship is gone. So what's your thoughts on it etc...
When you buy a ship it creates a room for the cockpit. This room is actually where you get memory left even if you use sellship. Since the room is still left in memory. I'm not sure if the room should be removed or not etc.... I found it by tossing in checks in STRALLOC and STRFREE to tell me what was getting allocated and freed and i knoticed that only 2 things between buyship and sellship weren't getting removed. One is a "" and the other was like room flags being put into room->flags. And so then I checked to see and found the room still stays there after the ship is gone. So what's your thoughts on it etc...
#24 Aug 28, 2009 8:10 pm
Off the Edge of the Map
GroupAdministrators
Posts1,200
JoinedMar 21, 2006
Delete the room, I would assume. If theres no ship associated with it, anymore, I'd say sell it.
#25 Aug 28, 2009 9:15 pm
Magician
GroupMembers
Posts239
JoinedJun 13, 2008
What about removeship or a ship being shot down? Also, when you say it stays there, what happens when another ship is created to the same vnum? From my understanding the room is still staying in the area file?
If so, that is very bad. As you can run out of vnums from ships being bought and sold.
Or you saying its staying in memory but not the file?
ayuri
If so, that is very bad. As you can run out of vnums from ships being bought and sold.
Or you saying its staying in memory but not the file?
ayuri
#26 Aug 28, 2009 9:20 pm
Geomancer
GroupAdministrators
Posts1,946
JoinedJul 26, 2005
Well as far as I can tell the room stays in the mud, when you copyover or anything after selling ships it will only create the ones it needs for ships that still exist. So on restart you will have sold ships cockpit room vnums available again. But aside from those it just keeps on going on vnums and keeping the rooms in the game.
Like take and buy 20 different ships and sell them as you go. then buy another ship and go to the cockpit room vnum and then start going back 1 vnum at a time and you will see all the cockpit rooms are still there that got created. Then take and hotboot and then start back at the ship that still exist vnum and try to go back you will notice they are gone. Only the one that still exist has a room. I for one wasn't sure if this was done for some reason or just an oversight but personally I agree with Kayle.
Like take and buy 20 different ships and sell them as you go. then buy another ship and go to the cockpit room vnum and then start going back 1 vnum at a time and you will see all the cockpit rooms are still there that got created. Then take and hotboot and then start back at the ship that still exist vnum and try to go back you will notice they are gone. Only the one that still exist has a room. I for one wasn't sure if this was done for some reason or just an oversight but personally I agree with Kayle.
#27 Aug 28, 2009 9:49 pm
Magician
GroupMembers
Posts239
JoinedJun 13, 2008
I'm not 100% sure I'm following. So I'm going to try to break it down and you can tell me if I'm right or wrong:
vnum 12001 first room for ship - cockpit of a Tie.
vnum 12002 first room for ship - cockpit of a r-41
vnum 12003 first room for ship - super star destroyer - cockpit is 12084
When I sell any of the ships in any order I see shipvnum.are cleaning out the rooms. No extra room is being left in the area file that I can see. Everything goes back to 12000 - 18000 for the vnum range.
Now I have seen something like what your saying happen before when the NUMROOM's don't match up to how many rooms are actually defined in the ship proto file. But then you start getting ships inside of other ships.
I guess I am looking for a bit more clarity, or a way to reproduce it (Or am I missing something here?) I have the latest swfotefuss a few days ago right after the new area format went in. I'll re-dl it and see if anything changes.
Is it only the cockpit flag? Maybe I need to work with larger ships. Or ships that only have the cockpit as the first room. I'll play about when I get home.
ayuri
vnum 12001 first room for ship - cockpit of a Tie.
vnum 12002 first room for ship - cockpit of a r-41
vnum 12003 first room for ship - super star destroyer - cockpit is 12084
When I sell any of the ships in any order I see shipvnum.are cleaning out the rooms. No extra room is being left in the area file that I can see. Everything goes back to 12000 - 18000 for the vnum range.
Now I have seen something like what your saying happen before when the NUMROOM's don't match up to how many rooms are actually defined in the ship proto file. But then you start getting ships inside of other ships.
I guess I am looking for a bit more clarity, or a way to reproduce it (Or am I missing something here?) I have the latest swfotefuss a few days ago right after the new area format went in. I'll re-dl it and see if anything changes.
Is it only the cockpit flag? Maybe I need to work with larger ships. Or ships that only have the cockpit as the first room. I'll play about when I get home.
ayuri
#28 Aug 28, 2009 10:08 pm
Geomancer
GroupAdministrators
Posts1,946
JoinedJul 26, 2005
Well go figure that time it removed them for me too.
Checks to see if the string is still leaking...and it is. So then we come on the fact that we need to free up the rooms correctly
So on to checking where it cleans up the room then lol
Checks to see if the string is still leaking...and it is. So then we come on the fact that we need to free up the rooms correctly
So on to checking where it cleans up the room then lol
#29 Aug 28, 2009 10:37 pm
Geomancer
GroupAdministrators
Posts1,946
JoinedJul 26, 2005
in ships.c
function make_prototype_rooms
find
change that to
find
change that to
These two are just a bad way to do it. First it was STRALLOCing it in and then picking it apart with one_argument.
in make_prototype_ship
find
change it to
Up near the top of the function it sets it to "" and then near the bottom it was setting it again without freeing the "".
function make_prototype_rooms
find
arg = STRALLOC( proom->flags );
change that to
arg = proom->flags;
find
rarg = STRALLOC( proom->reset[y] );
change that to
rarg = proom->reset[y];
These two are just a bad way to do it. First it was STRALLOCing it in and then picking it apart with one_argument.
in make_prototype_ship
find
ship->owner = STRALLOC( ch->name );
change it to
if( ship->owner ) STRFREE( ship->owner ); ship->owner = STRALLOC( ch->name );
Up near the top of the function it sets it to "" and then near the bottom it was setting it again without freeing the "".
#30 Aug 28, 2009 10:54 pm
Geomancer
GroupAdministrators
Posts1,946
JoinedJul 26, 2005
And btw that seems to for the most part fix up the buyship sellship leaking going on.
Once in awhile it will keep a link but then the next time or two it would be right back to where it started (like a delayed removal or something).
Once in awhile it will keep a link but then the next time or two it would be right back to where it started (like a delayed removal or something).
#31 Aug 29, 2009 12:00 pm
Last edited Aug 29, 2009 12:02 pm by Remcon
Geomancer
GroupAdministrators
Posts1,946
JoinedJul 26, 2005
Here is you some in act_info.c also.
in act_info.c
function do_hlist
find
change it to
find and remove
See the problem is that it does leak the idx if you were to do like "hlist test testing", because up where it does the STRALLOC it has an if( idx ) check and it returns without freeing it. It shouldn't need to be put into STRALLOCed anyways.
function do_setrank
find
change it to
It used STRALLOC and STRFREE when most places in the code use str_dup and DISPOSE.
find
change it to
It never DISPOSEd of it before setting it again.
function do_describe
find
change it to
the nfellow->knownas was just being set to = when it should have been STRALLOCed.
in act_info.c
function do_hlist
find
idx = STRALLOC( arg );
change it to
idx = arg;
find and remove
if( idx ) STRFREE( idx );
See the problem is that it does leak the idx if you were to do like "hlist test testing", because up where it does the STRALLOC it has an if( idx ) check and it returns without freeing it. It shouldn't need to be put into STRALLOCed anyways.
function do_setrank
find
if( !str_cmp( argument, "none" ) ) { if( vict->rank ) STRFREE( vict->rank ); vict->rank = STRALLOC( " " ); ch_printf( ch, "You have removed %s's rank.\r\n", PERS( vict, ch ) ); ch_printf( vict, "%s has removed your rank.\r\n", PERS( ch, vict ) ); return; }
change it to
if( !str_cmp( argument, "none" ) ) { if( vict->rank ) DISPOSE( vict->rank ); vict->rank = str_dup( " " ); ch_printf( ch, "You have removed %s's rank.\r\n", PERS( vict, ch ) ); ch_printf( vict, "%s has removed your rank.\r\n", PERS( ch, vict ) ); return; }
It used STRALLOC and STRFREE when most places in the code use str_dup and DISPOSE.
find
argument = smash_tilde_copy( argument ); // argument = rembg(argument); sprintf( buf, "%s", argument ); vict->rank = str_dup( buf );
change it to
argument = smash_tilde_copy( argument ); // argument = rembg(argument); sprintf( buf, "%s", argument ); if( vict->rank ) DISPOSE( vict->rank ); vict->rank = str_dup( buf );
It never DISPOSEd of it before setting it again.
function do_describe
find
ch_printf( ch, "You describe %s to %s.\r\n", fellow->knownas, PERS( victim, ch ) ); ch_printf( victim, "%s describes %s to you.\r\n", PERS( ch, victim ), fellow->knownas ); CREATE( nfellow, FELLOW_DATA, 1 ); nfellow->victim = QUICKLINK( victim->name ); nfellow->knownas = fellow->knownas; LINK( nfellow, victim->first_fellow, victim->last_fellow, next, prev ); return;
change it to
ch_printf( ch, "You describe %s to %s.\r\n", fellow->knownas, PERS( victim, ch ) ); ch_printf( victim, "%s describes %s to you.\r\n", PERS( ch, victim ), fellow->knownas ); CREATE( nfellow, FELLOW_DATA, 1 ); nfellow->victim = QUICKLINK( victim->name ); nfellow->knownas = STRALLOC( fellow->knownas ); LINK( nfellow, victim->first_fellow, victim->last_fellow, next, prev ); return;
the nfellow->knownas was just being set to = when it should have been STRALLOCed.
#32 Aug 29, 2009 12:25 pm
Last edited Sep 28, 2013 8:05 pm by Remcon
Geomancer
GroupAdministrators
Posts1,946
JoinedJul 26, 2005
in build.c
function do_cleanroom
change that to
It was just setting stuff and leaving all the data in memory.
Edit: wow had it using STRFREE on the name it had just set when it should have been description haha.
function do_cleanroom
void do_cleanroom( CHAR_DATA * ch, const char *argument ) { ROOM_INDEX_DATA *location; CHAR_DATA *vnext; CHAR_DATA *victim; OBJ_DATA *obj; OBJ_DATA *obj_next; EXIT_DATA *xit; int exitnum; location = ch->in_room; if( !ch->desc ) { send_to_char( "You have no descriptor.\r\n", ch ); return; } if( !can_rmodify( ch, location ) ) return; for( victim = ch->in_room->first_person; victim; victim = vnext ) { vnext = victim->next_in_room; if( IS_NPC( victim ) && victim != ch && !IS_SET( victim->act, ACT_POLYMORPHED ) ) extract_char( victim, TRUE ); } for( obj = ch->in_room->first_content; obj; obj = obj_next ) { obj_next = obj->next_content; if( obj->item_type == ITEM_SPACECRAFT ) continue; extract_obj( obj ); } for( exitnum = 0; exitnum < 11; exitnum++ ) { xit = get_exit( location, exitnum ); if( xit != NULL ) extract_exit( location, xit ); } location->first_extradesc = NULL; location->last_extradesc = NULL; location->name = STRALLOC( "Floating in a void" ); location->description = STRALLOC( "" ); location->room_flags = ROOM_PROTOTYPE; location->sector_type = 1; location->light = 0; ch_printf( ch, "&WRoom Cleared.\r\n" ); return; }
change that to
void do_cleanroom( CHAR_DATA * ch, const char *argument ) { ROOM_INDEX_DATA *location; EXTRA_DESCR_DATA *ed; CHAR_DATA *vnext; CHAR_DATA *victim; OBJ_DATA *obj; OBJ_DATA *obj_next; EXIT_DATA *xit; int exitnum; location = ch->in_room; if( !ch->desc ) { send_to_char( "You have no descriptor.\r\n", ch ); return; } if( !can_rmodify( ch, location ) ) return; for( victim = ch->in_room->first_person; victim; victim = vnext ) { vnext = victim->next_in_room; if( IS_NPC( victim ) && victim != ch && !IS_SET( victim->act, ACT_POLYMORPHED ) ) extract_char( victim, TRUE ); } for( obj = ch->in_room->first_content; obj; obj = obj_next ) { obj_next = obj->next_content; if( obj->item_type == ITEM_SPACECRAFT ) continue; extract_obj( obj ); } for( exitnum = 0; exitnum < 11; exitnum++ ) { xit = get_exit( location, exitnum ); if( xit != NULL ) extract_exit( location, xit ); } while( ( ed = location->first_extradesc ) != NULL ) { location->first_extradesc = ed->next; STRFREE( ed->keyword ); STRFREE( ed->description ); DISPOSE( ed ); --top_ed; } if( location->name ) STRFREE( location->name ); location->name = STRALLOC( "Floating in a void" ); if( location->description ) STRFREE( location->description ); location->description = STRALLOC( "" ); location->room_flags = ROOM_PROTOTYPE; location->sector_type = 1; location->light = 0; ch_printf( ch, "&WRoom Cleared.\r\n" ); return; }
It was just setting stuff and leaving all the data in memory.
Edit: wow had it using STRFREE on the name it had just set when it should have been description haha.
#33 Aug 29, 2009 12:42 pm
Geomancer
GroupAdministrators
Posts1,946
JoinedJul 26, 2005
in comm.c
function nanny_get_build
find
change that to
function nanny_get_droid
find
change it to
Better safe then sorry
function nanny_get_build
find
ch->comfreq = STRALLOC( buf );
change that to
if( ch->comfreq ) STRFREE( ch->comfreq ); ch->comfreq = STRALLOC( buf );
function nanny_get_droid
find
ch->comfreq = STRALLOC( buf );
change it to
if( ch->comfreq ) STRFREE( ch->comfreq ); ch->comfreq = STRALLOC( buf );
Better safe then sorry
#34 Aug 29, 2009 12:45 pm
Geomancer
GroupAdministrators
Posts1,946
JoinedJul 26, 2005
in comments.c
function do_comment
find
change that to
better safe then sorry.
function do_comment
find
ch->pnote->date = STRALLOC( strtime );
change that to
if( ch->pnote->date ) STRFREE( ch->pnote->date ); ch->pnote->date = STRALLOC( strtime );
better safe then sorry.
#35 Aug 29, 2009 12:52 pm
Geomancer
GroupAdministrators
Posts1,946
JoinedJul 26, 2005
in handler.c
function free_obj
find
change that to
Adding the armed_by so it frees that up also. (From the looks of things we will probably add more here later too).
function free_obj
find
STRFREE( obj->name ); STRFREE( obj->description ); STRFREE( obj->short_descr ); STRFREE( obj->action_desc ); DISPOSE( obj->killer ); DISPOSE( obj );
change that to
STRFREE( obj->name ); STRFREE( obj->description ); STRFREE( obj->short_descr ); STRFREE( obj->action_desc ); STRFREE( obj->armed_by ); DISPOSE( obj->killer ); DISPOSE( obj );
Adding the armed_by so it frees that up also. (From the looks of things we will probably add more here later too).
#36 Aug 29, 2009 4:28 pm
Geomancer
GroupAdministrators
Posts1,946
JoinedJul 26, 2005
in fight.c
function raw_kill
find
change it to
If your going to strip them and move the leaders around might as well do it fully.
function raw_kill
find
if( victim->pcdata && victim->pcdata->clan ) { if( victim->pcdata->clan->shortname && victim->pcdata->clan->shortname[0] != '\0' ) remove_member( victim->name, victim->pcdata->clan->shortname ); if( !str_cmp( victim->name, victim->pcdata->clan->leader ) ) { STRFREE( victim->pcdata->clan->leader ); if( victim->pcdata->clan->number1 ) { victim->pcdata->clan->leader = STRALLOC( victim->pcdata->clan->number1 ); STRFREE( victim->pcdata->clan->number1 ); victim->pcdata->clan->number1 = STRALLOC( "" ); } else if( victim->pcdata->clan->number2 ) { victim->pcdata->clan->leader = STRALLOC( victim->pcdata->clan->number2 ); STRFREE( victim->pcdata->clan->number2 ); victim->pcdata->clan->number2 = STRALLOC( "" ); } else victim->pcdata->clan->leader = STRALLOC( "" ); } if( !str_cmp( victim->name, victim->pcdata->clan->number1 ) ) { STRFREE( victim->pcdata->clan->number1 ); if( victim->pcdata->clan->number2 ) { victim->pcdata->clan->number1 = STRALLOC( victim->pcdata->clan->number2 ); STRFREE( victim->pcdata->clan->number2 ); victim->pcdata->clan->number2 = STRALLOC( "" ); } else victim->pcdata->clan->number1 = STRALLOC( "" ); } if( !str_cmp( victim->name, victim->pcdata->clan->number2 ) ) { STRFREE( victim->pcdata->clan->number2 ); victim->pcdata->clan->number1 = STRALLOC( "" ); } victim->pcdata->clan->members--; }
change it to
if( victim->pcdata && victim->pcdata->clan ) { if( victim->pcdata->clan->shortname && victim->pcdata->clan->shortname[0] != '\0' ) remove_member( victim->name, victim->pcdata->clan->shortname ); if( !str_cmp( victim->name, victim->pcdata->clan->leader ) ) { STRFREE( victim->pcdata->clan->leader ); if( victim->pcdata->clan->number1 ) { victim->pcdata->clan->leader = STRALLOC( victim->pcdata->clan->number1 ); STRFREE( victim->pcdata->clan->number1 ); victim->pcdata->clan->number1 = STRALLOC( "" ); if( victim->pcdata->clan->number2 ) { STRFREE( victim->pcdata->clan->number1 ); victim->pcdata->clan->number1 = STRALLOC( victim->pcdata->clan->number2 ); STRFREE( victim->pcdata->clan->number2 ); victim->pcdata->clan->number2 = STRALLOC( "" ); } } else if( victim->pcdata->clan->number2 ) { victim->pcdata->clan->leader = STRALLOC( victim->pcdata->clan->number2 ); STRFREE( victim->pcdata->clan->number2 ); victim->pcdata->clan->number2 = STRALLOC( "" ); } else victim->pcdata->clan->leader = STRALLOC( "" ); } if( !str_cmp( victim->name, victim->pcdata->clan->number1 ) ) { STRFREE( victim->pcdata->clan->number1 ); if( victim->pcdata->clan->number2 ) { victim->pcdata->clan->number1 = STRALLOC( victim->pcdata->clan->number2 ); STRFREE( victim->pcdata->clan->number2 ); victim->pcdata->clan->number2 = STRALLOC( "" ); } else victim->pcdata->clan->number1 = STRALLOC( "" ); } if( !str_cmp( victim->name, victim->pcdata->clan->number2 ) ) { STRFREE( victim->pcdata->clan->number2 ); victim->pcdata->clan->number1 = STRALLOC( "" ); } victim->pcdata->clan->members--; }
If your going to strip them and move the leaders around might as well do it fully.
#37 Aug 29, 2009 4:39 pm
Geomancer
GroupAdministrators
Posts1,946
JoinedJul 26, 2005
in force.c
function do_fset
find
change that to
function do_fhset
find
change it to
copy_buffer returns a string that has already been STRALLOCed so shouldn't use STRALLOC( copy_buffer( ch ) ); Each time it did that it was leaking strings into memory.
function do_fset
find
switch ( ch->substate ) { default: bug( "do_fset: illegal substate", 0 ); return; case SUB_RESTRICTED: send_to_char( "You cannot use this command from within another command.\r\n", ch ); return; case SUB_NONE: send_to_char( "USAGE: fset[ ]\r\r\n\n", ch ); send_to_char( "PUT VALID FIELDS HERE\r\n", ch ); return; case SUB_FORCE_CH0: fskill = ( FORCE_SKILL * ) ch->spare_ptr; STRFREE( fskill->ch_effect[0] ); fskill->ch_effect[0] = STRALLOC( copy_buffer( ch ) ); stop_editing( ch ); save_forceskill( fskill ); return; case SUB_FORCE_CH1: fskill = ( FORCE_SKILL * ) ch->spare_ptr; STRFREE( fskill->ch_effect[1] ); fskill->ch_effect[1] = STRALLOC( copy_buffer( ch ) ); stop_editing( ch ); save_forceskill( fskill ); return; case SUB_FORCE_CH2: fskill = ( FORCE_SKILL * ) ch->spare_ptr; STRFREE( fskill->ch_effect[2] ); fskill->ch_effect[2] = STRALLOC( copy_buffer( ch ) ); stop_editing( ch ); save_forceskill( fskill ); return; case SUB_FORCE_CH3: fskill = ( FORCE_SKILL * ) ch->spare_ptr; STRFREE( fskill->ch_effect[3] ); fskill->ch_effect[3] = STRALLOC( copy_buffer( ch ) ); stop_editing( ch ); save_forceskill( fskill ); return; case SUB_FORCE_CH4: fskill = ( FORCE_SKILL * ) ch->spare_ptr; STRFREE( fskill->ch_effect[4] ); fskill->ch_effect[4] = STRALLOC( copy_buffer( ch ) ); stop_editing( ch ); save_forceskill( fskill ); return; case SUB_FORCE_ROOM0: fskill = ( FORCE_SKILL * ) ch->spare_ptr; STRFREE( fskill->room_effect[0] ); fskill->room_effect[0] = STRALLOC( copy_buffer( ch ) ); stop_editing( ch ); save_forceskill( fskill ); return; case SUB_FORCE_ROOM1: fskill = ( FORCE_SKILL * ) ch->spare_ptr; STRFREE( fskill->room_effect[1] ); fskill->room_effect[1] = STRALLOC( copy_buffer( ch ) ); stop_editing( ch ); save_forceskill( fskill ); return; case SUB_FORCE_ROOM2: fskill = ( FORCE_SKILL * ) ch->spare_ptr; STRFREE( fskill->room_effect[2] ); fskill->room_effect[2] = STRALLOC( copy_buffer( ch ) ); stop_editing( ch ); save_forceskill( fskill ); return; case SUB_FORCE_ROOM3: fskill = ( FORCE_SKILL * ) ch->spare_ptr; STRFREE( fskill->room_effect[3] ); fskill->room_effect[3] = STRALLOC( copy_buffer( ch ) ); stop_editing( ch ); save_forceskill( fskill ); return; case SUB_FORCE_ROOM4: fskill = ( FORCE_SKILL * ) ch->spare_ptr; STRFREE( fskill->room_effect[4] ); fskill->room_effect[4] = STRALLOC( copy_buffer( ch ) ); stop_editing( ch ); save_forceskill( fskill ); return; case SUB_FORCE_VICTIM0: fskill = ( FORCE_SKILL * ) ch->spare_ptr; STRFREE( fskill->victim_effect[0] ); fskill->victim_effect[0] = STRALLOC( copy_buffer( ch ) ); stop_editing( ch ); save_forceskill( fskill ); return; case SUB_FORCE_VICTIM1: fskill = ( FORCE_SKILL * ) ch->spare_ptr; STRFREE( fskill->victim_effect[1] ); fskill->victim_effect[1] = STRALLOC( copy_buffer( ch ) ); stop_editing( ch ); save_forceskill( fskill ); return; case SUB_FORCE_VICTIM2: fskill = ( FORCE_SKILL * ) ch->spare_ptr; STRFREE( fskill->victim_effect[2] ); fskill->victim_effect[2] = STRALLOC( copy_buffer( ch ) ); stop_editing( ch ); save_forceskill( fskill ); return; case SUB_FORCE_VICTIM3: fskill = ( FORCE_SKILL * ) ch->spare_ptr; STRFREE( fskill->victim_effect[3] ); fskill->victim_effect[3] = STRALLOC( copy_buffer( ch ) ); stop_editing( ch ); save_forceskill( fskill ); return; case SUB_FORCE_VICTIM4: fskill = ( FORCE_SKILL * ) ch->spare_ptr; STRFREE( fskill->victim_effect[4] ); fskill->victim_effect[4] = STRALLOC( copy_buffer( ch ) ); stop_editing( ch ); save_forceskill( fskill ); return; }
change that to
switch ( ch->substate ) { default: bug( "do_fset: illegal substate", 0 ); return; case SUB_RESTRICTED: send_to_char( "You cannot use this command from within another command.\r\n", ch ); return; case SUB_NONE: send_to_char( "USAGE: fset[ ]\r\r\n\n", ch ); send_to_char( "PUT VALID FIELDS HERE\r\n", ch ); return; case SUB_FORCE_CH0: fskill = ( FORCE_SKILL * ) ch->spare_ptr; STRFREE( fskill->ch_effect[0] ); fskill->ch_effect[0] = copy_buffer( ch ); stop_editing( ch ); save_forceskill( fskill ); return; case SUB_FORCE_CH1: fskill = ( FORCE_SKILL * ) ch->spare_ptr; STRFREE( fskill->ch_effect[1] ); fskill->ch_effect[1] = copy_buffer( ch ); stop_editing( ch ); save_forceskill( fskill ); return; case SUB_FORCE_CH2: fskill = ( FORCE_SKILL * ) ch->spare_ptr; STRFREE( fskill->ch_effect[2] ); fskill->ch_effect[2] = copy_buffer( ch ); stop_editing( ch ); save_forceskill( fskill ); return; case SUB_FORCE_CH3: fskill = ( FORCE_SKILL * ) ch->spare_ptr; STRFREE( fskill->ch_effect[3] ); fskill->ch_effect[3] = copy_buffer( ch ); stop_editing( ch ); save_forceskill( fskill ); return; case SUB_FORCE_CH4: fskill = ( FORCE_SKILL * ) ch->spare_ptr; STRFREE( fskill->ch_effect[4] ); fskill->ch_effect[4] = copy_buffer( ch ); stop_editing( ch ); save_forceskill( fskill ); return; case SUB_FORCE_ROOM0: fskill = ( FORCE_SKILL * ) ch->spare_ptr; STRFREE( fskill->room_effect[0] ); fskill->room_effect[0] = copy_buffer( ch ); stop_editing( ch ); save_forceskill( fskill ); return; case SUB_FORCE_ROOM1: fskill = ( FORCE_SKILL * ) ch->spare_ptr; STRFREE( fskill->room_effect[1] ); fskill->room_effect[1] = copy_buffer( ch ); stop_editing( ch ); save_forceskill( fskill ); return; case SUB_FORCE_ROOM2: fskill = ( FORCE_SKILL * ) ch->spare_ptr; STRFREE( fskill->room_effect[2] ); fskill->room_effect[2] = copy_buffer( ch ); stop_editing( ch ); save_forceskill( fskill ); return; case SUB_FORCE_ROOM3: fskill = ( FORCE_SKILL * ) ch->spare_ptr; STRFREE( fskill->room_effect[3] ); fskill->room_effect[3] = copy_buffer( ch ); stop_editing( ch ); save_forceskill( fskill ); return; case SUB_FORCE_ROOM4: fskill = ( FORCE_SKILL * ) ch->spare_ptr; STRFREE( fskill->room_effect[4] ); fskill->room_effect[4] = copy_buffer( ch ); stop_editing( ch ); save_forceskill( fskill ); return; case SUB_FORCE_VICTIM0: fskill = ( FORCE_SKILL * ) ch->spare_ptr; STRFREE( fskill->victim_effect[0] ); fskill->victim_effect[0] = copy_buffer( ch ); stop_editing( ch ); save_forceskill( fskill ); return; case SUB_FORCE_VICTIM1: fskill = ( FORCE_SKILL * ) ch->spare_ptr; STRFREE( fskill->victim_effect[1] ); fskill->victim_effect[1] = copy_buffer( ch ); stop_editing( ch ); save_forceskill( fskill ); return; case SUB_FORCE_VICTIM2: fskill = ( FORCE_SKILL * ) ch->spare_ptr; STRFREE( fskill->victim_effect[2] ); fskill->victim_effect[2] = copy_buffer( ch ); stop_editing( ch ); save_forceskill( fskill ); return; case SUB_FORCE_VICTIM3: fskill = ( FORCE_SKILL * ) ch->spare_ptr; STRFREE( fskill->victim_effect[3] ); fskill->victim_effect[3] = copy_buffer( ch ); stop_editing( ch ); save_forceskill( fskill ); return; case SUB_FORCE_VICTIM4: fskill = ( FORCE_SKILL * ) ch->spare_ptr; STRFREE( fskill->victim_effect[4] ); fskill->victim_effect[4] = copy_buffer( ch ); stop_editing( ch ); save_forceskill( fskill ); return; }
function do_fhset
find
case SUB_FORCE_HELP: fhelp = ( FORCE_HELP * ) ch->spare_ptr; STRFREE( fhelp->desc ); fhelp->desc = STRALLOC( copy_buffer( ch ) ); stop_editing( ch ); save_forcehelp( fhelp ); return;
change it to
case SUB_FORCE_HELP: fhelp = ( FORCE_HELP * ) ch->spare_ptr; STRFREE( fhelp->desc ); fhelp->desc = copy_buffer( ch ); stop_editing( ch ); save_forcehelp( fhelp ); return;
copy_buffer returns a string that has already been STRALLOCed so shouldn't use STRALLOC( copy_buffer( ch ) ); Each time it did that it was leaking strings into memory.
#38 Aug 29, 2009 4:48 pm
Geomancer
GroupAdministrators
Posts1,946
JoinedJul 26, 2005
in fskills.c
funcion fskill_fdisguise
find
change that to
Most the stuff on here always keeps some data so best to free most of it before setting it.
function fskill_fhelp
find
change that to
Now argument is a const char *argument so not sure if it will like it or not but you shouldn't STRALLOC it thats for sure. It compiled in cygwin fine though after changing it.
funcion fskill_fdisguise
find
ch->pcdata->disguise = STRALLOC( argument );
change that to
if( ch->pcdata->disguise ) STRFREE( ch->pcdata->disguise ); ch->pcdata->disguise = STRALLOC( argument );
Most the stuff on here always keeps some data so best to free most of it before setting it.
function fskill_fhelp
find
if( !argument || argument[0] == '\0' ) argument = STRALLOC( "force" );
change that to
if( !argument || argument[0] == '\0' ) argument = "force";
Now argument is a const char *argument so not sure if it will like it or not but you shouldn't STRALLOC it thats for sure. It compiled in cygwin fine though after changing it.
#39 Aug 29, 2009 5:11 pm
Geomancer
GroupAdministrators
Posts1,946
JoinedJul 26, 2005
in ships.c
function do_buymobship
find
change to
function do_buymobship
find
ship->owner = STRALLOC( ch->pcdata->clan->name );
change to
if( ship->owner ) STRFREE( ship->owner ); ship->owner = STRALLOC( ch->pcdata->clan->name );
#40 Aug 29, 2009 5:28 pm
Last edited Sep 28, 2013 8:26 pm by Remcon
Geomancer
GroupAdministrators
Posts1,946
JoinedJul 26, 2005
in slicers.c
function do_tellsnoop
find
change that to
That should really be freed instead of just set to NULL.
find
change that to
It should free it if already exist.
Also noticed other issues with tell_snoop so might as well get them out the way now too.
in act_comm.c
find
change it to
farther down find
change it to
then find (yet another)
change that to
then find (yet another)
change that to
Edit: had ocj in one spot instead of och
function do_tellsnoop
find
if( !str_cmp( arg, "clear" ) || !str_cmp( arg, "self" ) ) { send_to_char( "You turn your radio off.\r\n", ch ); ch->pcdata->tell_snoop = NULL; return; }
change that to
if( !str_cmp( arg, "clear" ) || !str_cmp( arg, "self" ) ) { send_to_char( "You turn your radio off.\r\n", ch ); STRFREE( ch->pcdata->tell_snoop ); return; }
That should really be freed instead of just set to NULL.
find
if( number_percent( ) < schance ) { learn_from_success( ch, gsn_spy ); ch->pcdata->tell_snoop = STRALLOC( arg ); sprintf( buf, "You are now listening to all communications with %s.\r\n", ch->pcdata->tell_snoop ); send_to_char( buf, ch ); }
change that to
if( number_percent( ) < schance ) { learn_from_success( ch, gsn_spy ); if( ch->pcdata->tell_snoop ) STRFREE( ch->pcdata->tell_snoop ); ch->pcdata->tell_snoop = STRALLOC( arg ); sprintf( buf, "You are now listening to all communications with %s.\r\n", ch->pcdata->tell_snoop ); send_to_char( buf, ch ); }
It should free it if already exist.
Also noticed other issues with tell_snoop so might as well get them out the way now too.
in act_comm.c
find
if( och->pcdata->tell_snoop == ch->name && !IS_IMMORTAL( och ) && !IS_IMMORTAL( victim ) && !IS_IMMORTAL( ch ) )
change it to
if( och->pcdata->tell_snoop && !str_cmp( och->pcdata->tell_snoop, ch->name ) && !IS_IMMORTAL( och ) && !IS_IMMORTAL( victim ) && !IS_IMMORTAL( ch ) )
farther down find
if( och->pcdata->tell_snoop == victim->name && !IS_IMMORTAL( och ) && !IS_IMMORTAL( victim ) && !IS_IMMORTAL( ch ) )
change it to
if( och->pcdata->tell_snoop && !str_cmp( och->pcdata->tell_snoop, victim->name ) && !IS_IMMORTAL( och ) && !IS_IMMORTAL( victim ) && !IS_IMMORTAL( ch ) )
then find (yet another)
if( och->pcdata->tell_snoop == ch->name && !IS_IMMORTAL( och ) && !IS_IMMORTAL( victim ) && !IS_IMMORTAL( ch ) )
change that to
if( och->pcdata->tell_snoop && !str_cmp( och->pcdata->tell_snoop, ch->name ) && !IS_IMMORTAL( och ) && !IS_IMMORTAL( victim ) && !IS_IMMORTAL( ch ) )
then find (yet another)
if( och->pcdata->tell_snoop == victim->name && !IS_IMMORTAL( och ) && !IS_IMMORTAL( victim ) && !IS_IMMORTAL( ch ) )
change that to
if( och->pcdata->tell_snoop && !str_cmp( och->pcdata->tell_snoop, victim->name ) && !IS_IMMORTAL( och ) && !IS_IMMORTAL( victim ) && !IS_IMMORTAL( ch ) )
Edit: had ocj in one spot instead of och