Problems with "Skinning"

Posted by RavenSavage on Sat 11 Aug 2007 08:48 AM — 11 posts, 39,474 views.

RavenSavage USA Sat 11 Aug 2007 08:48 AM #0

Howdy,

I'm trying to change the skin code around and have managed to compile my changes and whatnot, but when the skill is used, there is a crash. Below is the code with changes, I have a feeling I'm not freeing/allocating something properly, any help would be appreciated.


void do_skin( CHAR_DATA *ch, char *argument )
{
   OBJ_DATA *korps, *corpse, *obj, *skin;
   bool checkknife;
   char *name;
   char buf[MAX_STRING_LENGTH];
   int schance;
   
   switch ( ch->substate )
   {
      default:
            
        if( argument[0] == '\0' )
        {
          send_to_char( "What corpse do you wish to skin?\r\n", ch );
          return;
        }
   
        checkknife = FALSE;
          
   
        if( ( corpse = get_obj_here( ch, argument ) ) == NULL )
        {
          send_to_char( "You cannot find that here.\r\n", ch );
          return;
        }
         
     for ( obj = ch->last_carrying; obj; obj = obj->prev_content )
        {
          if ( obj->item_type == ITEM_SKNIFE && obj->wear_loc == WEAR_WIELD )
          checkknife = TRUE;
        }
        
        if ( !checkknife )
        {
          send_to_char( "You are not wielding a skinning knife.\r\n", ch );
          return;
        }

        if( corpse->value[4] == 25)
        {
          send_to_char( "There is no skin on this corpse.\r\n", ch );
          return;
        }
        if( get_obj_index( OBJ_VNUM_SKIN ) == NULL )
        {
          bug( "Vnum %d (OBJ_VNUM_SKIN) not found for do_skin!", OBJ_VNUM_SKIN );
          return;
        }
   
    schance = IS_NPC(ch) ? ch->level : (int) (ch->pcdata->learned[gsn_skin]) ;

        if ( number_percent( ) < schance )
        {
           send_to_char( "&GYou begin the process of skinning a corpse.\n\r", ch);
           act( AT_PLAIN, "$n takes $s knife and begins to skin a corpse.", ch, NULL, argument , TO_ROOM );
           add_timer ( ch , TIMER_DO_FUN , 2 , do_skin , 1 );
           ch->alloc_ptr = str_dup( buf );
           return;
         }
           send_to_char("&RYour knife slips and ruins the skin.\n\r",ch);
           learn_from_failure( ch, gsn_skin );
           corpse->value[4] = 25;
           return;
           
      case 1:
         if( !ch->alloc_ptr )
         return;
         mudstrlcpy( buf, ch->alloc_ptr, MAX_INPUT_LENGTH );
         DISPOSE( ch->alloc_ptr );
         break;
         
      case SUB_TIMER_DO_ABORT:
         DISPOSE( ch->alloc_ptr );
         ch->substate = SUB_NONE;
         send_to_char( "You carefully stop what you were doing.\r\n", ch );
         return;
   }

    ch->substate = SUB_NONE;           
   
   korps = create_object( get_obj_index( OBJ_VNUM_CORPSE_NPC ), 0 );
   skin = create_object( get_obj_index( OBJ_VNUM_SKIN ), 0 );
   name = IS_NPC( ch ) ? korps->short_descr : corpse->short_descr;
   snprintf( buf, MAX_STRING_LENGTH, skin->short_descr, name );
   STRFREE( skin->short_descr );
   skin->short_descr = STRALLOC( buf );
   snprintf( buf, MAX_STRING_LENGTH, skin->description, name );
   STRFREE( skin->description );
   skin->description = STRALLOC( buf );
   corpse->value[4] = 25;
   act( AT_BLOOD, "$n strips the skin from $p.", ch, corpse, NULL, TO_ROOM );
   act( AT_BLOOD, "You strip the skin from $p.", ch, corpse, NULL, TO_CHAR );
   obj_to_char( skin, ch );
   return;
}

Any help is appreciated.

Nick Gammon Australia Forum Administrator Sat 11 Aug 2007 10:36 AM #1

Short of trying it myself, I suggest you fire up gdb and make it crash - at least that shows what line is responsible.

Nick Gammon Australia Forum Administrator Sat 11 Aug 2007 10:45 AM #2

I can see one problem, you have introduced a switch statement, compared to the original. If the ch->substate is equal to 1, then the variable corpse is not defined, however you dereference it later on.

Isthiriel Sat 11 Aug 2007 11:55 AM #3

if ( number_percent( ) < schance )
{
  send_to_char( "&GYou begin the process of skinning a corpse.\n\r", ch);
  act( AT_PLAIN, "$n takes $s knife and begins to skin a corpse.", ch, NULL, argument , TO_ROOM );
  add_timer ( ch , TIMER_DO_FUN , 2 , do_skin , 1 );
  ch->alloc_ptr = str_dup( buf );
  return;
}

If I'm reading it correctly, buf hasn't been initialised at this point? So, if it doesn't contain a 0, str_dup will run off the end of the array and segfault.

What's supposed to happen to the korps object? It looks like it's being leaked at the end of the function?

Gohan_TheDragonball USA Sun 12 Aug 2007 02:28 AM #4

move:
if( ( corpse = get_obj_here( ch, argument ) ) == NULL )
{
send_to_char( "You cannot find that here.\r\n", ch );
return;
}

to just before the substate switch statement. like nick said you are referencing its corpse->short_descr later when it is only being initialized when the command is first typed. moving that to before the switch statement should fix your problem, also it will prevent a crash if in the time it takes the timer to expire someone picks up the corpse.

Gohan_TheDragonball USA Sun 12 Aug 2007 02:31 AM #5

and just as a possible catch:
STRFREE( skin->short_descr );
STRFREE( skin->description );

should be
if ( skin->short_descr )
STRFREE( skin->short_descr );
if ( skin->description )
STRFREE( skin->description );

RavenSavage USA Sun 12 Aug 2007 05:16 AM #6

Moving the corpse check to before the switch statement does prevent a crash, but shorts out the timer (as in, when a character issues a command in the timer, the abort doesn't fire, it instantly completes the whole thing)

Also, it doesn't move past the corpse check, when the timer completes, it gives the "You cannot find that here" message, as per the check. So, the check apparently isn't working right. I'll fiddle with it some more. (though it's looking more appealing to kill the whole switch and just make it a delay)

I appreciate all the help.

Isthiriel Sun 12 Aug 2007 06:23 AM #7

Quote:
should be

if ( skin->short_descr )
  STRFREE( skin->short_descr );
if ( skin->description )
  STRFREE( skin->description );

snprintf with a NULL cause a segfault before we reach the STRFREE. So, if you are going to add the sanity check (and how would short_descr or description end up NULL anyway?), it has to be added higher:

if (skin->short_descr) {
  snprintf( buf, MAX_STRING_LENGTH, skin->short_descr, name );
  STRFREE( skin->short_descr );
  skin->short_descr = STRALLOC( buf );
}
if (skin->description) {
  snprintf( buf, MAX_STRING_LENGTH, skin->description, name );
  STRFREE( skin->description );
  skin->description = STRALLOC( buf );
}

Conner USA Sun 12 Aug 2007 10:18 PM #8

For what it's worth, this is how I made my do_skin work to my satisfaction (Thanks to Remcon for his help with this one) a couple of years ago, maybe it'll work for you too?

void do_skin( CHAR_DATA * ch, char *argument )
{
   OBJ_DATA *corpse, *obj, *skin;
   char buf[MAX_STRING_LENGTH], *temp;
   if( !IS_PKILL( ch ) && !IS_IMMORTAL( ch ) )
   {
      send_to_char( "Leave the hideous defilings to the killers!\n", ch );
      return;
   }
   if( !argument || argument[0] == '\0' )
   {
      send_to_char( "Whose corpse do you wish to skin?\r\n", ch );
      return;
   }
   if( ( corpse = get_obj_here( ch, argument ) ) == NULL )
   {
      send_to_char( "You cannot find that here.\r\n", ch );
      return;
   }
   if( !IS_IMMORTAL( ch ) )
   {
      /* This is restrictions for mortals only 
       * They have to hold a weapon and of a certian type
       * They can only skin other player corpses and thats if they got final blow
       * And the corpse can only be skinned once       */
      if( ( obj = get_eq_char( ch, WEAR_WIELD ) ) == NULL )
      {
         send_to_char( "You have no weapon with which to perform this deed.\r\n", ch );
         return;
      }
      if( obj->value[3] != 1 && obj->value[3] != 2 && obj->value[3] != 3 && obj->value[3] != 11 )
      {
         send_to_char( "You're not holding the right kind of weapon to skin a corpse.\r\n", ch );
         return;
      }
      if( corpse->item_type != ITEM_CORPSE_PC )
      {
         send_to_char( "You can only skin the bodies of player characters.\r\n", ch );
         return;
      }
      if( !corpse->action_desc || str_cmp( corpse->action_desc, ch->name ) )
      {
         send_to_char( "You can't skin a corpse if you didn't get the final blow on the player.\r\n", ch );
         return;
      }
      if( corpse->value[5] >= 1 )
      {
         send_to_char( "That corpse has already been skinned.\r\n", ch );
         return;
      }
   }
   /* Immortals can skin npc and pc corpses */
   else if( corpse->item_type != ITEM_CORPSE_PC && corpse->item_type != ITEM_CORPSE_NPC )
   {
      send_to_char( "You can only skin corpses.\r\n", ch );
      return;
   }
   if( !get_obj_index( OBJ_VNUM_SKIN ) )
   {
      bug( "%s: Vnum %d (OBJ_VNUM_SKIN) not found!", __FUNCTION__, OBJ_VNUM_SKIN );
      send_to_char( "The skin object couldn't be found.\r\n", ch );
      return;
   }
   if( !( skin = create_object( get_obj_index( OBJ_VNUM_SKIN ), 0 ) ) )
   {
      bug( "%s: couldn't create skin [%d]", __FUNCTION__, OBJ_VNUM_SKIN );
      send_to_char( "A skin object couldn't be created.\r\n", ch );
      return;
   }
   temp = corpse->short_descr;
   temp = one_argument( temp, buf );
   temp = one_argument( temp, buf );
   temp = one_argument( temp, buf );
   if( skin->short_descr )
   {
      snprintf( buf, MAX_STRING_LENGTH, skin->short_descr, temp );
      STRFREE( skin->short_descr );
      skin->short_descr = STRALLOC( buf );
   }
   if( skin->description )
   {
      snprintf( buf, MAX_STRING_LENGTH, skin->description, temp );
      STRFREE( skin->description );
      skin->description = STRALLOC( buf );
   }
   corpse->value[5] = 1;
   act( AT_BLOOD, "$n strips the skin from $p.", ch, corpse, NULL, TO_ROOM );
   act( AT_BLOOD, "You strip the skin from $p.", ch, corpse, NULL, TO_CHAR );
   obj_to_char( skin, ch );
   return;
}

Gohan_TheDragonball USA Mon 13 Aug 2007 02:54 AM #9

sorry i guess i should have mentioned a couple things. it makes sense that when the timer expires, it does not find the corpse anywhere since you are not storing the argument. also when i went to check if you were storing the argument, i found this:


        if ( number_percent( ) < schance )
        {
           send_to_char( "&GYou begin the process of skinning a corpse.\n\r", ch);
           act( AT_PLAIN, "$n takes $s knife and begins to skin a corpse.", ch, NULL, argument , TO_ROOM );
           add_timer ( ch , TIMER_DO_FUN , 2 , do_skin , 1 );
           ch->alloc_ptr = str_dup( buf );
           return;
         }

as of this point you have yet to set buf to anything, i assume you meant:
ch->alloc_ptr = str_dup( argument );

so try the following:


void do_skin( CHAR_DATA *ch, char *argument )
{
   OBJ_DATA *korps, *corpse, *obj, *skin;
   bool checkknife;
   char *name;
   char buf[MAX_STRING_LENGTH];
   int schance;
   
   switch ( ch->substate )
   {
      default:
            
        if( argument[0] == '\0' )
        {
          send_to_char( "What corpse do you wish to skin?\r\n", ch );
          return;
        }
   
        checkknife = FALSE;
          
        if( ( corpse = get_obj_here( ch, argument ) ) == NULL )
        {
          send_to_char( "You cannot find that here.\r\n", ch );
          return;
        }
         
     	for ( obj = ch->last_carrying; obj; obj = obj->prev_content )
        {
          if ( obj->item_type == ITEM_SKNIFE && obj->wear_loc == WEAR_WIELD )
          checkknife = TRUE;
        }
        
        if ( !checkknife )
        {
          send_to_char( "You are not wielding a skinning knife.\r\n", ch );
          return;
        }

        if( corpse->value[4] == 25)
        {
          send_to_char( "There is no skin on this corpse.\r\n", ch );
          return;
        }
        if( get_obj_index( OBJ_VNUM_SKIN ) == NULL )
        {
          bug( "Vnum %d (OBJ_VNUM_SKIN) not found for do_skin!", OBJ_VNUM_SKIN );
          return;
        }
   
    	schance = IS_NPC(ch) ? ch->level : (int) (ch->pcdata->learned[gsn_skin]) ;

        if ( number_percent( ) < schance )
        {
           send_to_char( "&GYou begin the process of skinning a corpse.\n\r", ch);
           act( AT_PLAIN, "$n takes $s knife and begins to skin a corpse.", ch, NULL, argument , TO_ROOM );
           add_timer ( ch , TIMER_DO_FUN , 2 , do_skin , 1 );
           ch->alloc_ptr = str_dup( argument );
           return;
         }
           send_to_char("&RYour knife slips and ruins the skin.\n\r",ch);
           learn_from_failure( ch, gsn_skin );
           corpse->value[4] = 25;
           return;
           
      case 1:
         if( !ch->alloc_ptr )
         return;
         mudstrlcpy( buf, ch->alloc_ptr, MAX_INPUT_LENGTH );
         DISPOSE( ch->alloc_ptr );
         break;
         
      case SUB_TIMER_DO_ABORT:
         DISPOSE( ch->alloc_ptr );
         ch->substate = SUB_NONE;
         send_to_char( "You carefully stop what you were doing.\r\n", ch );
         return;
   }

    ch->substate = SUB_NONE; 
          
    if( ( corpse = get_obj_here( ch, buf ) ) == NULL )
    {
      send_to_char( "You cannot find that here.\r\n", ch );
      return;
    }

   korps = create_object( get_obj_index( OBJ_VNUM_CORPSE_NPC ), 0 );
   skin = create_object( get_obj_index( OBJ_VNUM_SKIN ), 0 );
   name = IS_NPC( ch ) ? korps->short_descr : corpse->short_descr;
   snprintf( buf, MAX_STRING_LENGTH, skin->short_descr, name );
   STRFREE( skin->short_descr );
   skin->short_descr = STRALLOC( buf );
   snprintf( buf, MAX_STRING_LENGTH, skin->description, name );
   STRFREE( skin->description );
   skin->description = STRALLOC( buf );
   corpse->value[4] = 25;
   act( AT_BLOOD, "$n strips the skin from $p.", ch, corpse, NULL, TO_ROOM );
   act( AT_BLOOD, "You strip the skin from $p.", ch, corpse, NULL, TO_CHAR );
   obj_to_char( skin, ch );
   return;
}

RavenSavage USA Mon 13 Aug 2007 04:23 AM #10

Beautiful, works like a charm.

Much thanks to all.