Login
User Name:

Password:



Register

Forgot your password?
Changes list / Addchange
Author: Khonsu
Submitted by: Khonsu
6Dragons mp3 sound pack
Author: Vladaar
Submitted by: Vladaar
AFKMud 2.2.3
Author: AFKMud Team
Submitted by: Samson
SWFOTEFUSS 1.5
Author: Various
Submitted by: Samson
SWRFUSS 1.4
Author: Various
Submitted by: Samson
Users Online
Google, Bing

Members: 0
Guests: 21
Stats
Files
Topics
Posts
Members
Newest Member
488
3,788
19,631
595
Khonsu

Today's Birthdays
There are no member birthdays today.
» SmaugMuds » Codebases » SmaugFUSS » STRFREE on strdup fight. C
Forum Rules | Mark all | Recent Posts

STRFREE on strdup fight. C
< Newer Topic :: Older Topic >

Pages:<< prev 1, 2 next >>
Post is unread #1 Mar 6, 2018 7:15 am   
Go to the top of the page
Go to the bottom of the page

Vladaar
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.
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

Post is unread #2 Mar 6, 2018 4:20 pm   
Go to the top of the page
Go to the bottom of the page

GatewaySysop
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.

Post is unread #3 Mar 6, 2018 7:00 pm   Last edited Mar 6, 2018 7:15 pm by Vladaar
Go to the top of the page
Go to the bottom of the page

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.
#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 =

Post is unread #4 Mar 6, 2018 10:34 pm   
Go to the top of the page
Go to the bottom of the page

GatewaySysop
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.

Post is unread #5 Mar 7, 2018 5:01 pm   
Go to the top of the page
Go to the bottom of the page

Remcon
Geomancer
GroupAdministrators
Posts1,914
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.

Post is unread #6 Mar 7, 2018 5:51 pm   Last edited Mar 7, 2018 5:52 pm by GatewaySysop
Go to the top of the page
Go to the bottom of the page

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?

Post is unread #7 Mar 7, 2018 8:56 pm   
Go to the top of the page
Go to the bottom of the page

Vladaar
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);
      }

Post is unread #8 Mar 7, 2018 9:54 pm   
Go to the top of the page
Go to the bottom of the page

GatewaySysop
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.

Post is unread #9 Mar 8, 2018 4:39 pm   
Go to the top of the page
Go to the bottom of the page

Vladaar
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;
}

Post is unread #10 Mar 8, 2018 8:12 pm   Last edited Mar 8, 2018 8:16 pm by GatewaySysop
Go to the top of the page
Go to the bottom of the page

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.




Post is unread #11 Mar 8, 2018 8:16 pm   Last edited Mar 8, 2018 8:18 pm by Vladaar
Go to the top of the page
Go to the bottom of the page

Vladaar
Apprentice
GroupMembers
Posts57
JoinedNov 24, 2016

 
#define QUICKLINK(point) quick_link( (point), __FILE__, __LINE__ )

Post is unread #12 Mar 8, 2018 8:22 pm   
Go to the top of the page
Go to the bottom of the page

GatewaySysop
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?

Post is unread #13 Mar 8, 2018 8:39 pm   Last edited Mar 8, 2018 8:44 pm by Vladaar
Go to the top of the page
Go to the bottom of the page

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

Post is unread #14 Mar 8, 2018 11:55 pm   Last edited Mar 8, 2018 11:56 pm by GatewaySysop
Go to the top of the page
Go to the bottom of the page

GatewaySysop
Conjurer
GroupMembers
Posts413
JoinedMar 7, 2005

 
A little perplexed. Why does this part from your GDB not match the code you posted earlier?

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?



Post is unread #15 Mar 9, 2018 6:14 am   
Go to the top of the page
Go to the bottom of the page

Vladaar
Apprentice
GroupMembers
Posts57
JoinedNov 24, 2016

 
No. It's the same.

Post is unread #16 Mar 9, 2018 11:18 am   
Go to the top of the page
Go to the bottom of the page

GatewaySysop
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.

Post is unread #17 Mar 9, 2018 3:04 pm   
Go to the top of the page
Go to the bottom of the page

Vladaar
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.

Post is unread #18 Mar 9, 2018 3:27 pm   Last edited Mar 9, 2018 3:31 pm by GatewaySysop
Go to the top of the page
Go to the bottom of the page

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.

Post is unread #19 Mar 9, 2018 4:55 pm   
Go to the top of the page
Go to the bottom of the page

Vladaar
Apprentice
GroupMembers
Posts57
JoinedNov 24, 2016

 
Ok, thanks for clarifying. You might be right there, well seems only likely explanation.

Post is unread #20 Mar 13, 2018 1:26 am   Last edited Mar 13, 2018 1:27 am by GatewaySysop
Go to the top of the page
Go to the bottom of the page

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?

Pages:<< prev 1, 2 next >>