STRFREE on strdup fight. C
< Newer Topic
:: Older Topic >
#1 Mar 6, 2018 7:15 am
Apprentice
GroupMembers
Posts57
JoinedNov 24, 2016
I am getting a crash core now and then from fight.c due to STRFREE being used on on a strdup.
It's this part here.
I am thinking I might have an issue with this Because I have mobile hate level so mobs change who they hate based on damage amounts.
I wonder instead why this isn't written using the functions created instead of the above....
Use
That would apply the Dispose in the function
It's this part here.
if( dam && npcvict && ch != victim ) { if( !xIS_SET( victim->act, ACT_SENTINEL ) ) { if( victim->hunting ) { if( victim->hunting->who != ch ) { STRFREE( victim->hunting->name ); victim->hunting->name = QUICKLINK( ch->name ); victim->hunting->who = ch; } } else if( !xIS_SET( victim->act, ACT_PACIFIST ) ) /* Gorog */ start_hunting( victim, ch ); } if( victim->hating ) { if( victim->hating->who != ch ) { STRFREE( victim->hating->name ); victim->hating->name = QUICKLINK( ch->name ); victim->hating->who = ch; } } else if( !xIS_SET( victim->act, ACT_PACIFIST ) ) /* Gorog */ start_hating( victim, ch ); }
I am thinking I might have an issue with this Because I have mobile hate level so mobs change who they hate based on damage amounts.
I wonder instead why this isn't written using the functions created instead of the above....
Use
Stop_hating(victim) ; Start_hating(victim, ch) ;
That would apply the Dispose in the function
#2 Mar 6, 2018 4:20 pm
Conjurer
GroupMembers
Posts413
JoinedMar 7, 2005
I'm a bit confused, maybe you should post your stop/start functions here too? I've never encountered an issue with this and, at least looking in what I thought was still stock functions on my end, I don't see what difference it would make.
#3 Mar 6, 2018 7:00 pm
Last edited Mar 6, 2018 7:15 pm by Vladaar
Apprentice
GroupMembers
Posts57
JoinedNov 24, 2016
Gateway I know, surprised myself.
But this why I think it. First my log file showed this at point of crash
Mon Mar 5 22:25:07 2018 :: &RSTRFREE called on str_dup pointer: fight.c, line 2972&D
I picked that up in my log, because I have this defined in mud.h which I copied from Remcon's LOP as seemed like a great idea to pick up memory issues.
My core showed the below.​ without the emoticon smiley faces.
But this why I think it. First my log file showed this at point of crash
Mon Mar 5 22:25:07 2018 :: &RSTRFREE called on str_dup pointer: fight.c, line 2972&D
I picked that up in my log, because I have this defined in mud.h which I copied from Remcon's LOP as seemed like a great idea to pick up memory issues.
#define STRFREE(point) \ do \ { \ if( (point) ) \ { \ if( !in_hash_table( (point) ) ) \ { \ log_printf( "&RSTRFREE called on str_dup pointer: %s, line %d&D\n", __FILE__, __LINE__ ); \ free( (point) ); \ } \ else \ str_free( (point), __FILE__, __LINE__ ); \ } \ (point) = NULL; \ } while(0)
My core showed the below.​ without the emoticon smiley faces.
#0 0x00007f8441aed428 in __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:54
#1 0x00007f8441aef02a in __GI_abort () at abort.c:89
#2 0x00007f8441b2f7ea in __libc_message (do_abort=do_abort@entry=2, fmt=fmt@entry=0x7f8441c48ed8 "*** Error in `%s': %s: 0x%s ***\n")
at ../sysdeps/posix/libc_fatal.c:175
#3 0x00007f8441b3837a in malloc_printerr (ar_ptr=, ptr= , str=0x7f8441c48fe8 "double free or corruption (out)",
action=3) at malloc.c:5006
#4 _int_free (av=, p= , have_lock=0) at malloc.c:3867
#5 0x00007f8441b3c53c in __GI___libc_free (mem=) at malloc.c:2968
#6 0x0000000000504433 in damage (ch=ch@entry=0x28d3420, victim=victim@entry=0x23961f0, dam=57, dt=dt@entry=377) at fight.c:2972
#7 0x00000000005b67b4 in do_kick (ch=0x28d3420, argument=) at skills.c:3453
#8 0x00000000005b2daf in check_skill (ch=ch@entry=0x28d3420, command=command@entry=0x7ffc3729ce00 "kick",
argument=argument@entry=0xe2db04 "") at skills.c:493
#9 0x000000000052403a in interpret (ch=ch@entry=0x28d3420, argument=0xe2db04 "") at interp.c:397
#10 0x00000000004a882a in do_combo (ch=0x28d3420, argument=) at combo.c:45
#11 0x00000000005b2daf in check_skill (ch=ch@entry=0x28d3420, command=command@entry=0x7ffc372a0710 "combo",
argument=argument@entry=0x7ffc372a1f95 "") at skills.c:493
#12 0x000000000052403a in interpret (ch=0x28d3420, argument=0x7ffc372a1f95 "", argument@entry=0x7ffc372a1f90 "combo") at interp.c:397
#13 0x00000000004a5a92 in game_loop () at comm.c:1184
#14 0x00000000004a5fc0 in main (argc=, argv= ) at comm.c:814
(gdb) frame 6
#6 0x0000000000504433 in damage (ch=ch@entry=0x28d3420, victim=victim@entry=0x23961f0, dam=57, dt=dt@entry=377) at fight.c:2972
2972 STRFREE(victim->hating->name);
(gdb) list
2967 if(victim->hating)
2968 {
2969 if(victim->hating->who != ch)
2970 {
2971 if ( victim->hating )
2972 STRFREE(victim->hating->name);
2973 if ( ch ) {
2974 victim->hating->name = QUICKLINK(ch->name);
2975 victim->hating->who = ch;
2976 }
(gdb) print victim->name
$1 = 0x13cdbc0 "beaver"
(gdb) print victim->hating->name
$2 = 0x2787350 "corpse A timber-0"
(gdb) print ch->name
$3 = 0x1f8b800 "Nasdak"
(gdb) info locals
damobj =
pObj =
gch =
buf = "\311\305)7\374\177\000\000\240\t\223\000\000\000\000\000\260D\031\002\000\000\000\000\000\060\314t@\001xc\000\000\000\000\000\000\000\000\360Óˆ\002\000\000\000\000\071\000\000\000\000\000\000\000'\000J\000\000\000\000\000\360Óˆ\002\000\000\000\000\071", '\000', "\237\260I\000\000\000\000\000 4\215\002\000\000\000\000\300\260I", '\000' , "\264\264I\000\000\000\000\000-\304f\000\000\000\000\000\030\000\000\000\060\000\000\000P\346)7\374\177\000\000\220\345)7\374\177\000\000You perform a combo atack hitting first wi"...
buf1 = "\311\305)7\374\177\000\000\240\t\223\000\000\000\000\000\260D\031\002\000\000\000\000\000\060\314t@\001xc\000\000\000\000\000\000\000\000\360Óˆ\002\000\000\000\000\071\000\000\000\000\000\000\000'\000J\000\000\000\000\000\360Óˆ\002\000\000\000\000\071", '\000', "\237\260I\000\000\000\000\000 4\215\002\000\000\000\000\300\260I", '\000' , "\264\264I\000\000\000\000\000-\304f\000\000\000\000\000\030\000\000\000\060\000\000\000P\346)7\374\177\000\000\220\345)7\374\177\000\000You perform a combo atack hitting first wi"...
dameq =
#4 Mar 6, 2018 10:34 pm
Conjurer
GroupMembers
Posts413
JoinedMar 7, 2005
Vladaar said:
Gateway I know, surprised myself.
But this why I think it. First my log file showed this at point of crash
Mon Mar 5 22:25:07 2018 :: &RSTRFREE called on str_dup pointer: fight.c, line 2972&D
I picked that up in my log, because I have this defined in mud.h which I copied from Remcon's LOP as seemed like a great idea to pick up memory issues.
I'm guessing you didn't have any issues prior to porting it over? I'm not sure what differences are with LOP, but I'd not be surprised if it isn't entirely compatible with stock. There might be something else you need to port over. Maybe Remcon can shed more light on this, but that's my $.02.
#5 Mar 7, 2018 5:01 pm
Geomancer
GroupAdministrators
Posts1,917
JoinedJul 26, 2005
you should take a look and make sure that quick link is using stralloc etc...
That part shouldn't have any issues btw, it just helps with finding some issues.
That part shouldn't have any issues btw, it just helps with finding some issues.
#6 Mar 7, 2018 5:51 pm
Last edited Mar 7, 2018 5:52 pm by GatewaySysop
Conjurer
GroupMembers
Posts413
JoinedMar 7, 2005
Vladaar said:
I am getting a crash core now and then from fight.c due to STRFREE being used on on a strdup.
I am thinking I might have an issue with this Because I have mobile hate level so mobs change who they hate based on damage amounts.
Can you show us how you did that piece? Maybe that is the part of the puzzle missing here. The change from Remcon's LOP and the code you show here should make no difference that I can see. Maybe the code is breaking here because of somewhere else having to do with how you're managing the hate level and the changing of who they hate?
#7 Mar 7, 2018 8:56 pm
Apprentice
GroupMembers
Posts57
JoinedNov 24, 2016
if(ch->fighting && victim != ch) for(gch = victim->in_room->first_person; gch; gch = gch->next_in_room) if(is_same_group(victim, gch) && (victim != gch) && gch->hate_level > victim->hate_level) { stop_fighting(ch, FALSE); /* Stop the current fighting for ch only */ start_hating(ch, gch ); set_fighting(ch, gch); /* Start them on the next one? */ snprintf(buf, MSL, "\r\n&O Yell [%s] %s you will die for that!", ch->name, gch->name); do_recho(ch, buf); }
#8 Mar 7, 2018 9:54 pm
Conjurer
GroupMembers
Posts413
JoinedMar 7, 2005
How is your QUICKLINK defined in mud.h? I still don't see anything jumping out at me to explain how the mismatch is taking place.
#9 Mar 8, 2018 4:39 pm
Apprentice
GroupMembers
Posts57
JoinedNov 24, 2016
char *quick_link(const char *str, const char *filename, int line) { register struct hashstr_data *ptr; if(!str || str[0] == '\0') { // bug( "%s: %s@%d trying to allocate an empty/null string", __FUNCTION__, filename, line ); return NULL; } ptr = (struct hashstr_data *)(str - sizeof(struct hashstr_data)); if(!ptr || ptr->links == 0) { fprintf(stderr, "%s: %s@%d called bad pointer\n", __FUNCTION__, filename, line); return NULL; } if((ptr->links + 1) > 65500) return str_alloc(str, filename, line); else ++ptr->links; return (char *)str; }
#10 Mar 8, 2018 8:12 pm
Last edited Mar 8, 2018 8:16 pm by GatewaySysop
Conjurer
GroupMembers
Posts413
JoinedMar 7, 2005
Vladaar said:char *quick_link(const char *str, const char *filename, int line) {}
GatewaySysop said:
How is your QUICKLINK defined in mud.h?
Note the distinction and which file is being talked about.
If QUICKLINK is using quick_link( ) which is using str_alloc, why are you getting an error staying it was str_dup( )'ed and now attempting to STRFREE it? There's a disconnect here, hence the question about how QUICKLINK is defined in mud.h, because if it isn't using quick_link then maybe that's related to your issue.
#11 Mar 8, 2018 8:16 pm
Last edited Mar 8, 2018 8:18 pm by Vladaar
Apprentice
GroupMembers
Posts57
JoinedNov 24, 2016
#define QUICKLINK(point) quick_link( (point), __FILE__, __LINE__ )
#12 Mar 8, 2018 8:22 pm
Conjurer
GroupMembers
Posts413
JoinedMar 7, 2005
Vladaar said:
#define QUICKLINK(point) quick_link( (point), __FILE__, __LINE__ )
I don't know about yours, but mine is conditional and I thought it was in stock. Maybe you should show what surrounds this?
I seem to recall that if not using certain defines, it will fall back on using str_dup rather than hashing strings, won't it?
Something like this:
#ifdef HASHSTR #define STRALLOC(point) str_alloc((point)) #define QUICKLINK(point) quick_link((point)) #define STRFREE(point) \ do \ { \\ } while(0) #else #define STRALLOC(point) str_dup((point)) #define QUICKLINK(point) str_dup((point)) #define STRFREE(point) DISPOSE((point)) #endif
Do you see what I'm getting at? Are you sure your code is actually using quick_link( ) as it should or are the defines mismatched somehow?
#13 Mar 8, 2018 8:39 pm
Last edited Mar 8, 2018 8:44 pm by Vladaar
Apprentice
GroupMembers
Posts57
JoinedNov 24, 2016
Should be copied and pasted from LOP when I ported it. It's almost exactly same as smaugfuss cept has more debugging info like file, line in bug messages whereas smaugfuss has some of it, but not quite as much.
#ifdef HASHSTR #define STRALLOC(point) str_alloc( (point), __FILE__, __LINE__ ) #define QUICKLINK(point) quick_link( (point), __FILE__, __LINE__ ) #define STRFREE(point) \ do \ { \ if( (point) ) \ { \ if( !in_hash_table( (point) ) ) \ { \ log_printf( "&RSTRFREE called on str_dup pointer: %s, line %d&D\n", __FILE__, __LINE__ ); \ free( (point) ); \ } \ else \ str_free( (point), __FILE__, __LINE__ ); \ } \ (point) = NULL; \ } while(0) #else #define STRALLOC( point ) str_dup( (point) ) #define QUICKLINK( point ) str_dup( (point) ) #define STRFREE( point ) STRDISPOSE( (point) ) #endif
#14 Mar 8, 2018 11:55 pm
Last edited Mar 8, 2018 11:56 pm by GatewaySysop
Conjurer
GroupMembers
Posts413
JoinedMar 7, 2005
A little perplexed. Why does this part from your GDB not match the code you posted earlier?
vs.
Something seems not to match between the code you show and the code that caused the crash. Did you change it?
2967 if(victim->hating) 2968 { 2969 if(victim->hating->who != ch) 2970 { 2971 if ( victim->hating ) 2972 STRFREE(victim->hating->name); 2973 if ( ch ) { 2974 victim->hating->name = QUICKLINK(ch->name); 2975 victim->hating->who = ch; 2976 }
vs.
if( victim->hating ) { if( victim->hating->who != ch ) { STRFREE( victim->hating->name ); victim->hating->name = QUICKLINK( ch->name ); victim->hating->who = ch; } }
Something seems not to match between the code you show and the code that caused the crash. Did you change it?
#15 Mar 9, 2018 6:14 am
Apprentice
GroupMembers
Posts57
JoinedNov 24, 2016
No. It's the same.
#16 Mar 9, 2018 11:18 am
Conjurer
GroupMembers
Posts413
JoinedMar 7, 2005
Vladaar said:
No. It's the same.
How can you make the assertion that the two above pieces of code are the same? Clearly they are not. I really want to help here and understand what's wrong, but I can't debug one piece of code when GDB was clearly looking at something else.
#17 Mar 9, 2018 3:04 pm
Apprentice
GroupMembers
Posts57
JoinedNov 24, 2016
I'm not lying, I have no reason too. I'm not the best coder, and will not pretend to be. I posted cause I thought it was a legit bug or at least an issue with my code.
#18 Mar 9, 2018 3:27 pm
Last edited Mar 9, 2018 3:31 pm by GatewaySysop
Conjurer
GroupMembers
Posts413
JoinedMar 7, 2005
Vladaar said:
I'm not lying, I have no reason too. I'm not the best coder, and will not pretend to be. I posted cause I thought it was a legit bug or at least an issue with my code.
Nobody is saying you're lying. What I'm asking is why GDB's log is showing a different version of that block of code from what you posted initially. Notice the if ( ch ) check and redundant if( victim->hating ) checks. Where did those go?
Your problem could well be that ch didn't exist when that conditional was run, so while it STRFREE'ed victim->hating->name, it didn't put anything in its place. Your GDB log seems to indicate "corpse" as a name so that's another reason I'm suspecting something is off. Think about it, next go around, maybe trying to STRFREE something already freed (and not QUICKLINK'ed again afterwards) is the source of your initial error.
Don't take it the wrong way, but as I said, difficult to debug when code that created the error is not the code that was posted initially.
#19 Mar 9, 2018 4:55 pm
Apprentice
GroupMembers
Posts57
JoinedNov 24, 2016
Ok, thanks for clarifying. You might be right there, well seems only likely explanation.
#20 Mar 13, 2018 1:26 am
Last edited Mar 13, 2018 1:27 am by GatewaySysop
Conjurer
GroupMembers
Posts413
JoinedMar 7, 2005
Vladaar said:
Ok, thanks for clarifying. You might be right there, well seems only likely explanation.
Were you able to figure out if anything else was going on with this, or confirm what was causing the crash?