I'm saving player data to my accounts and getting garbage saved under the clan variable.

#PLAYER
Name    Matthis~
Level   100
Clan    Àå@     h98     ^Ðñå·^Ðñå·^Øñå·^Øñå· ñå· ñå·¨ñå·¨ñå·°ñå·°ñå·¸ñå·¸ñå·Àñå·Àñå·(w% ¨^Ò@    ^HH8    ^ØR8    Øñå·Øñå·àñå·àñå· V9      V9     ðñå·ðñå·øñå·øñå·~
Created 1280033744
LogOff  1280497022
LogOn  1280491969
Logons 3
Room    300
End

I do get this error occasionally but I don't understand how it's happening:

Log: [*****] BUG: STRFREEing bad pointer in save.cpp, line 178

It's updated like so during save:

   if( !IS_NPC(ch) && ch->pcdata->account )
   {
      PLAYER_DATA *player = NULL;

      if( (player = getPlayer(ch->name)))
      {
         player->logOff = current_time;
         player->level = ch->top_level;
         player->room = ch->in_room->vnum;
         if( ch->pcdata->clan )
         {
            if( player->clan )
               STRFREE( player->clan );
            player->clan = STRALLOC( ch->pcdata->clan->name );
         }
         else
         {
            if( player->clan )              
               STRFREE( player->clan ); // LINE 178
            player->clan = STRALLOC( "" );
         }
         if( ch->pcdata->account->lastPlayed )
            STRFREE( ch->pcdata->account->lastPlayed );
         ch->pcdata->account->lastPlayed = STRALLOC(ch->name );
      }
      else
        bug( "%s: no player data for %s?", __FUNCTION__, ch->name );
      ch->pcdata->account->lastOn = current_time;
      SaveAccount(ch->pcdata->account);   
   } 

And saved like so:

            if( (*iPlyr)->clan && (*iPlyr)->clan[0] != '\0' )
               fprintf( fp, "Clan    %s~\n", (*iPlyr)->clan );

It sometimes crashes when it saves and loads due to the clan variable, and some clan variables on players are screwed up, but with the same garbage. Any idea how to fix this and why it's doing it?

mud@mud:~/swgi/accounts$ grep Clan *
xxxxx:Clan    The Galactic Empire~
xxxxx:Clan    The Galactic Empire~
xxxxx:Clan    The Galactic Empire~
xxxxx:Clan    The Galactic Empire~
xxxxx:Clan    The Jedi Order~
xxxxx:Clan    Àå@       <8      <8      <8      Ø@      Ø@       ñå· ñå·¨ñå·¨ñå·èY8     èY8     ¸ñå·¸ñå·Àñå·Àñå·Èñå·Èñå·R8      R8      Øñå·Øñå·àñå·àñå· V9      V9     ðñå·ðñå·øñå·øñå·~
xxxxx:Clan    The Galactic Empire~
xxxxx:Clan    The Galactic Empire~
xxxxx:Clan    The Jedi Order~
xxxxx:Clan    The Jedi Order~
xxxxx:Clan    Àå@     <8      <8      <8      Ø@      Ø@       ñå· ñå·¨ñå·¨ñå·èY8     èY8     ¸ñå·¸ñå·Àñå·Àñå·Èñå·Èñå·R8      R8      Øñå·Øñå·àñå·àñå· V9      V9     ðñå·ðñå·øñå·øñå·~
Burrium:Clan    The Galactic Empire~
xxxxx:Clan    Àå@    <8      <8      <8      Ø@      Ø@       ñå· ñå·¨ñå·¨ñå·èY8     èY8     ¸ñå·¸ñå·Àñå·Àñå·Èñå·Èñå·R8      R8      Øñå·Øñå·àñå·àñå· V9      V9     ðñå·ðñå·øñå·øñå·~
xxxxx:Clan    Àå@   <8      <8      <8      Ø@      Ø@       ñå· ñå·¨ñå·¨ñå·èY8     èY8     ¸ñå·¸ñå·Àñå·Àñå·Èñå·Èñå·R8      R8      Øñå·Øñå·àñå·àñå· V9      V9     ðñå·ðñå·øñå·øñå·~
xxxxx:Clan    The Galactic Empire~
xxxxx:Clan    The Galactic Empire~
xxxxx:Clan    Àå@       <8      <8      <8      Ø@      Ø@       ñå· ñå·¨ñå·¨ñå·èY8     èY8     ¸ñå·¸ñå·Àñå·Àñå·Èñå·Èñå·R8      R8      Øñå·Øñå·àñå·àñå· V9      V9     ðñå·ðñå·øñå·øñå·~
xxxxx:Clan    The Galactic Empire~
xxxxx:Clan    Àå@     <8      <8      <8      Ø@      Ø@       ñå· ñå·¨ñå·¨ñå·èY8     èY8     ¸ñå·¸ñå·Àñå·Àñå·Èñå·Èñå·R8      R8      Øñå·Øñå·àñå·àñå· V9      V9     ðñå·ðñå·øñå·øñå·~
xxxxx:Clan    Àå@       <8      <8      <8      Ø@      Ø@       ñå· ñå·¨ñå·¨ñå·èY8     èY8     ¸ñå·¸ñå·Àñå·Àñå·Èñå·Èñå·R8      R8      Øñå·Øñå·àñå·àñå· V9      V9     ðñå·ðñå·øñå·øñå·~
xxxxx:Clan    The Jedi Order~
xxxxx:Clan    The Jedi Order~
xxxxx:Clan    The Jedi Order~
xxxxx:Clan    The Jedi Order~
xxxxx:Clan    The Galactic Empire~
xxxxxClan    Àå@     h98     å·å·ñå·ñå· ñå· ñå·¨ñå·¨ñå·°ñå·°ñå·¸ñå·¸ñå·Àñå·Àñå·(w%   ¨@     H8       R8      Øñå·Øñå·àñå·àñå· V9      V9     ðñå·ðñå·øñå·øñå·~
xxxxx:Clan    The Galactic Empire~
xxxxx:Clan    Àå@     <8      <8      <8      Ø@      Ø@       ñå· ñå·¨ñå·¨ñå·èY8     èY8     ¸ñå·¸ñå·Àñå·Àñå·Èñå·Èñå·R8      R8      Øñå·Øñå·àñå·àñå· V9      V9     ðñå·ðñå·øñå·øñå·~
xxxxxClan    The Jedi Order~
xxxxx:Clan    Blacksun~
xxxxx:Clan    Àå@     <8      <8      <8      Ø@      Ø@       ñå· ñå·¨ñå·¨ñå·èY8     èY8     ¸ñå·¸ñå·Àñå·Àñå·Èñå·Èñå·R8      R8      
Øñå·Øñå·àñå·àñå· V9      V9     ðñå·ðñå·øñå·øñå·~
xxxxx:Clan    Àå@  <8      <8      <8      Ø@      Ø@       ñå· ñå·¨ñå·¨ñå·èY8     èY8     ¸ñå·¸ñå·Àñå·Àñå·Èñå·Èñå·R8      R8      Øñå·Øñå·àñå·àñå· V9      V9     ðñå·ðñå·øñå·øñå·~
xxxxx:Clan    Àñå·Àñå·Èñå·Èñå·R8   R8      Øñå·Øñå·àñå·àñå· V9      V9     ðñå·ðñå·øñå·øñå·~
xxxxx:Clan    Blacksun~
mud@mud:~/swgi/accounts$

Note: The xxxx's are just obscured account names. 

You need to double check that the clan names aren't being assigned using str_dup anywhere. They all need to use the same. if you want to use STRALLOC/STRFREE on the clan_name fields, then ALL changes to clan_name need to use STRALLOC/STRFREE, not just the accounts part of things.

         if( ch->pcdata->clan )
         {
            if( player->clan )
               STRFREE( player->clan );
            player->clan = STRALLOC( ch->pcdata->clan->name );
         }
         else
         {
            if( player->clan )              
               STRFREE( player->clan ); // LINE 178
            player->clan = STRALLOC( "" );
         }

Instead of doing it like that I would do it like this

         player->clan = NULL;
         if( ch->pcdata->clan )
            player->clan = STRALLOC( ch->pcdata->clan->name );
         else
            player->clan = STRALLOC( "" );

More or less sometimes player->clan hadn't been set with anything so it might have had junk data which ment it wasn't NULL to start with so it tried to free it so it gave you the bug message. Nothing had been assigned to it yet anyways so just set it NULL then set the data or set "". Should fix the bug and might fix the junk data being put in there also.

Kayle said:

You need to double check that the clan names aren't being assigned using str_dup anywhere. They all need to use the same. if you want to use STRALLOC/STRFREE on the clan_name fields, then ALL changes to clan_name need to use STRALLOC/STRFREE, not just the accounts part of things.

Everywhere I access the player->clan variable I use STRFREE/ALLOC. Does that mean this is bad (can I not smash an str_dupped variable into a STRALLOC'd variable)?

ch->pcdata->clan->name = str_dup(bob);
player->clan = STRALLOC(ch->pcdata->clan->name);

What's the difference between str_dup/dispose and the STRFREE/ALLOC anyway?

Remcon said:

code

More or less sometimes player->clan hadn't been set with anything so it might have had junk data which ment it wasn't NULL to start with so it tried to free it so it gave you the bug message. Nothing had been assigned to it yet anyways so just set it NULL then set the data or set "". Should fix the bug and might fix the junk data being put in there also.

I've applied this fix and hopefully it will clean some things up, but I was loading player->clan like so:

         case 'E':                 
            if( !str_cmp( word, "End" ) )
            {                             
               if( !player->clan || player->clan[0] == '\0' )
                 player->clan = STRALLOC("");  
               account->alts++;                  
               allPlayers.push_back(player);  
               account->playerList.push_back(player);
               player->account = account;
               return;
            }
            break;

I have, however, modified my freadPlayer code so that player->clan is set to NULL before it is read so it will either have something assigned or be set to a blank.

EDIT: Editted for formatting and clarification.

What's the difference between str_dup/dispose and the STRFREE/ALLOC anyway?

STRFREE and STRALLOC use a shared string mechanism (using reference counting) whereas str_dup and dispose don't. So, dispose will delete memory immediately even if other strings are using it; strfree on a non-shared pointer will look for reference count data that doesn't exist.

David Haley said:

What's the difference between str_dup/dispose and the STRFREE/ALLOC anyway?

STRFREE and STRALLOC use a shared string mechanism (using reference counting) whereas str_dup and dispose don't. So, dispose will delete memory immediately even if other strings are using it; strfree on a non-shared pointer will look for reference count data that doesn't exist.

What's an example of a situation where str_dup would be favored over STRALLOC?

Shared strings introduce a (very small) amount of overhead during creation and freeing, and take up a (very small) amount of extra memory. Additionally, shared strings are, well, shared, so you can't just modify it as you please: you need to create a temporary copy, modify it, and then put it back into the shared string system.

So you would not use a shared string if:
- you were very concerned about every last byte of memory and processor instruction (the chances of this being true are next to nil)
- you wanted to have a string you own and can quickly/easily modify

All of this is made easier with a proper shared string API, though; the SMAUG shared string API kind of stinks. (It's too easy to confuse shared strings with non-shared strings.)

It really isn't to bad and I long ago decided to just put everything in it instead of using both shared and non shared strings to make it easier to keep track of.