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
Bing, AhrefsBot, Sogou

Members: 0
Guests: 27
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 » AFKMud Release Announcements » AFKMud 1.56b Released
Forum Rules | Mark all | Recent Posts

AFKMud 1.56b Released
< Newer Topic :: Older Topic > Last of the 1.5 series

Pages:<< prev 1 next >>
Post is unread #1 Sep 23, 2003 12:55 pm   
Go to the top of the page
Go to the bottom of the page

Samson
Black Hand
GroupAdministrators
Posts3,685
JoinedJan 1, 2002

 
Version 1.56b of AFKMud has been released.

This is a bugfix/maintenance release. This will also be the last release in the 1.5 series.
Future changes to the code are extensive enough to make further patches infeasible.

Standard disclaimer type stuff: Changes in this version may or may not remain compatible with your
older support files, such as areas, commands, skills, socials, etc. If things break, you were warned.

A new patching run was used in this version that should catch more than just source code
changes. To apply this patch to a stock base, drop it into the main afkmud directory and
type:

patch -p1 < 156bpatch

This will update all the necessary files. Be advised that doing so in a non-stock setup may
result in things you weren't counting on. You should read this changelog thoroughly before
applying it.

Bugfixes this release:

All changes for Intermud-3 2.25 Client included. [Samson]
Missed a removal for the Fortify debugger in mud.h [Samson]

Non-Code related changes:

Updated helps for authorize, think, and change form. [Vyn]
Updated helps for I3. [Samson]
Fixed a targetting bug on the invisibility spell. [Samson]

Post is unread #2 Sep 24, 2003 2:27 pm   Last edited Nov 24, 2007 2:25 am by Samson
Go to the top of the page
Go to the bottom of the page

Quixadhal
Conjurer
GroupMembers
Posts398
JoinedMar 8, 2005

 
Heh, by taunting us with "Last of the 1.5 series", I happened upon this little buglet in magic.c, around line 5599:
         act( AT_MAGIC, "You age horribly!", ch, NULL, victim, TO_VICT );
         act( AT_MAGIC, "%n ages horribly!", victim, NULL, NULL, TO_ROOM );
         victim->pcdata->age_bonus += 10;

I believe you want a $n there, not a %n.

Post is unread #3 Sep 24, 2003 3:04 pm   
Go to the top of the page
Go to the bottom of the page

Samson
Black Hand
GroupAdministrators
Posts3,685
JoinedJan 1, 2002

 
Yes, you are quite right. Fixed, but don't expect to see it before the overhaul is complete.

BTW, thanks for the MSL/MIL patch. Used it, and was glad to have it since it would have taken much time away from other things just to go fix those.

Also - regarding your color bleed patch. Resorted to a much simpler method. Terminal reset code sent with each prompt line. That seemed far less aggressive than the insertion into the other code you had, and so far it seems to be working out nicely this way.

Jury is still out on the strlcpy/strlcat patch. May opt to use the same less complicated code the I3 client is using. It appears to do the same thing. Also, some of the comments you left in there were somewhat uncertain. Spots where you didn't know what was going on, so when the time comes, that audit will need to be done by hand.

Post is unread #4 Sep 24, 2003 3:41 pm   
Go to the top of the page
Go to the bottom of the page

Quixadhal
Conjurer
GroupMembers
Posts398
JoinedMar 8, 2005

 
My pleasure. I'm currently about halfway through nailing down all the sprintf calls and making them become snprintf, or adding vararg support to whatever used to need the sprintf/temp buffer. That's a little more "contravertial" change, but it doesn't add much overhead (it actually reduces memory footprint by having far fewer temp buffers), and makes the code quite a bit simpler.

I'll shoot you a copy of that when I finish, as always, to use or not as you like

I chose strlcpy mainly because I didn't like strncpy's "usually" NULL terminated behaviour. In my mind, strings have to always be NULL terminated, or they aren't really strings. For strlcat, it was more the readability of always passing the buffer size, rather than BUFLEN-strlen-1.

I am curious, does strlcpy exist in FreeBSD (I had to use my test machine to replace a dead power supply box, so I can't check until I reinstall it on something else).

At least one of the places I didn't mess with looked like it intentially copied a fraction of a string into the middle of another string (hence you would NOT want a terminating NULL added).

If any of this results in a more stable beastie, then I'm happy

Post is unread #5 Sep 24, 2003 4:22 pm   Last edited Nov 24, 2007 2:26 am by Samson
Go to the top of the page
Go to the bottom of the page

Samson
Black Hand
GroupAdministrators
Posts3,685
JoinedJan 1, 2002

 
Yes, just checked with a FreeBSD server I have access to. strlcpy and strlcat are both there. They are not present in Cygwin or Linux though.

My understanding of the licensing involved is that BSD code is compatible with Diku licensing. While you may be able to charge for BSD licensed products, you are also allowed to use portions of them in product which payment is forbidden for. So I may just go ahead and run the audit on those.

Already hacked away at WAY too much to use the patch. No doubt many hunks would fail :P

If it becomes an issue of what's best, how does something like this look to you?

void i3strncpy( char *dest, const char *src, size_t size )
{
   strncpy( dest, src, size-1 );
   dest[size-1] = '�';
}

Post is unread #6 Sep 24, 2003 8:47 pm   Last edited Nov 24, 2007 2:26 am by Samson
Go to the top of the page
Go to the bottom of the page

Quixadhal
Conjurer
GroupMembers
Posts398
JoinedMar 8, 2005

 
strlcpy/strlcat return the number of characters that you wanted to copy, which allows for overflow detection by checking if retlen > len (in case that matters, perhaps in data storage or OLC routines).

If you want to use the more common strncpy/strncat routines without looping through the source string once (as the strl family does), you can do so, but it will have slightly (function call) more overhead. To also detect overflows, you'd have to call strlen, which would be not quite double the cost. In the case of strncat, you have to call strlen twice (once to see how much space is left, and once if you want the overflow check).

In any case, you'll want to add a few security checks to that, to handle bogus pointers and such:

size_t i3strncpy(char *dst, const char *src, size_t len)
{
  if(!src || !dst || len < 1)
    return 0;
  if(!*src) {
    *dst = '�';
    return 0;
  }
  strncpy(dst, src, len-1);
  dst[len-1] = '�';
  return strlen(dst); // Change to 1 to only report success.
}

size_t i3strncat(char *dst, const char *src, size_t len)
{
  register int dlen;

  if(!src || !*src || !dst || len = len-1) // already over
    return 0;
  strncat(dst, src, len - dlen - 1);
  dst[len-1] = '�';
  return strlen(dst); // Change to 1 to only report success.
}

Post is unread #7 Oct 1, 2003 1:58 pm   Last edited Nov 24, 2007 2:26 am by Samson
Go to the top of the page
Go to the bottom of the page

Quixadhal
Conjurer
GroupMembers
Posts398
JoinedMar 8, 2005

 
I think I found one more little bug in fight.c
    WAIT_STATE( ch, 1 * sysdata.pulseviolence );
  sprintf( buf, "Help!  I am being attacked by %s!", IS_NPC( ch ) ? ch->short_descr : ch->name );
  if ( !IS_PKILL(victim) )
      interpret( victim, buf );

around line 4036, in do_murder, I believe that wants to be something more like "shout Help! I am....". The murder verb may not be used very often, so this might not have really shown up.

Post is unread #8 Oct 3, 2003 2:06 pm   Last edited Nov 24, 2007 2:26 am by Samson
Go to the top of the page
Go to the bottom of the page

Quixadhal
Conjurer
GroupMembers
Posts398
JoinedMar 8, 2005

 
and... one more little memory leak bug...
in tables.c, there's an fread_string to a temp variable around line 1603, it needs to have a companion STRFREE before it goes out of scope:
     case 'F':
           if( !str_cmp( word, "flags" ) )
           {
               if( version flags = fread_number( fp );
                  fMatch = TRUE;
                  break;
               }
               else
               {
                  char *cmdflags = NULL;
                char flag[MIL];
                int value;
                                                                                                                   
/* here */    cmdflags = fread_string( fp );
                                                                                                                   
                while ( cmdflags[0] != '�' )
                {
                   cmdflags = one_argument( cmdflags, flag );
                   value = get_cmdflag( flag );
                   if ( value = 32 )
                       bug( "Unknown command flag: %s", flag );
                   else
                       TOGGLE_BIT( command->flags, 1 << value );
                }
                                                                                                                   
/* here */    STRFREE(cmdflags);
                fMatch = TRUE;
                break;
                                                                                                                   
             }
         }
         break;


Of course, they may still get handles to them elsewhere, since I'm sure those words get used all over the olc code and such too, but at least this ref to them should be clean.

Post is unread #9 Oct 9, 2003 2:01 pm   Last edited Nov 24, 2007 2:26 am by Samson
Go to the top of the page
Go to the bottom of the page

Quixadhal
Conjurer
GroupMembers
Posts398
JoinedMar 8, 2005

 
Here's a possible bug in the item_ego() routine of act_obj.c:
   if( obj_ego >= 100000 )
               obj_ego = 90;
  else
               obj_ego /= 1000;
                                                                                                       
   return(obj_ego);

The code above seems to want to cap the return value at 90, but if obj_ego is between 91000 and 99999, it will return 91..99 as a value.
Is that intended? It seems like you'd either want it to return 90 for anything >= 90000, or return 99 for anything >= 99000 (or possibly 100 >= 100000 if you want "perfect" scores).

Post is unread #10 Oct 9, 2003 2:30 pm   
Go to the top of the page
Go to the bottom of the page

Samson
Black Hand
GroupAdministrators
Posts3,685
JoinedJan 1, 2002

 
The fight.c thing is moot. do_murder was taken out and it's PK checks consolidated into do_kill.

The object ego calc is correct, look further up. The variable obj_ego is set to obj->rent before getting to where you're looking.

The memory leak in tables.c:

==22811== 61112 bytes in 1923 blocks are still reachable in loss record 5 of 6
==22811== at 0x4002989A: calloc (vg_replace_malloc.c:273)
==22811== by 0x80FA4E0: str_alloc (hashstr.c:52)
==22811== by 0x80D7EEF: fread_string (db.c:545)
==22811== by 0x8160128: fread_command (tables.c:1588)

This is after having done what you suggest. I've been after this one for eons. If you look at the stuff in db.c for where it loads flags, you'll see that section is the same setup. So why does just this one complain? And why doesn't adding the STRFREE( cmdflags ); solve it? I would dearly love to slay this leak, but it isn't letting me

Post is unread #11 Oct 10, 2003 8:07 am   
Go to the top of the page
Go to the bottom of the page

Quixadhal
Conjurer
GroupMembers
Posts398
JoinedMar 8, 2005

 
I know obj_ego gets initialized to whatever obj->rent has, but what I'm puzzled by is why it gets clipped to 90 if it's over 100K, but from 91K to 99K can be 91 through 99. In other words, why not clip it at 100, or why not do the clipping for > 90K?

IOW: Why should a weapon worth 95K have a 95 ego, but a weapon worth 500K has only a 90 ego?

Hmmm, as to the memory leak... I'm assuming something else has a lock on those shared string buffers. SInce many of the commands are common words, the odds are good that some of them will end up being str_alloc'd elsewhere too. We could make that particular copy go away by changing it to a str_dup/DISPOSE pair, but the other one would (in theory) still be floating out there. If it DID go away, that means there's only one hanging reference elsewhere... which might be findable (as opposed to possibly many copies).

Post is unread #12 Oct 10, 2003 6:37 pm   
Go to the top of the page
Go to the bottom of the page

Samson
Black Hand
GroupAdministrators
Posts3,685
JoinedJan 1, 2002

 
Point taken about the ego thing. Don't know why it was capped that way. 90K it is.

As for tables.c, changed to fread_string_nohash/DISPOSE

Got the following:

Fri Oct 10, 2003 4:31:18 PM PDT :: Loading commands...
==10517== Invalid free() / delete / delete[]
==10517== at 0x400296D7: free (vg_replace_malloc.c:220)
==10517== by 0x815E21B: fread_command (tables.c:1599)
==10517== by 0x815E3EB: load_commands (tables.c:1702)
==10517== by 0x80DACE9: boot_db (db.c:2081)
==10517== Address 0x41A04B6A is 6 bytes inside a block of size 7 alloc'd
==10517== at 0x4002989A: calloc (vg_replace_malloc.c:273)
==10517== by 0x80E0055: str_dup (db.c:5373)
==10517== by 0x80D80B7: fread_string_nohash (db.c:618)
==10517== by 0x815E1D0: fread_command (tables.c:1588)

At which point it offers to attach to GDB for debugging. Examination reveals what's expected, the cmdflags variable is empty at that time.

Releasing it to continue loading completes bootup.

Shutdown produces a new report of the results though, and exposes a new set underneath:

==10517== 2585 bytes in 375 blocks are definitely lost in loss record 5 of 7
==10517== at 0x4002989A: calloc (vg_replace_malloc.c:273)
==10517== by 0x80E0055: str_dup (db.c:5373)
==10517== by 0x80D80B7: fread_string_nohash (db.c:618)
==10517== by 0x815E1D0: fread_command (tables.c:1588)

==10517== 55527 bytes in 1548 blocks are still reachable in loss record 6 of 7
==10517== at 0x4002989A: calloc (vg_replace_malloc.c:273)
==10517== by 0x80FA5B4: str_alloc (hashstr.c:52)
==10517== by 0x80D7FAB: fread_string (db.c:545)
==10517== by 0x80670C4: fread_shellcommand (shell.c:1265)

So I think your theory that it's allocating tons of junk to the string hash is probably right.

Post is unread #13 Oct 10, 2003 9:30 pm   
Go to the top of the page
Go to the bottom of the page

Samson
Black Hand
GroupAdministrators
Posts3,685
JoinedJan 1, 2002

 
*Feindish laugh from the 8th level of hell*

DEATH TO LEAKS!

*Ahem*

There. That felt better. It seems I've managed a solution. Copied fread_string and named the copy fread_flagstring. Behaved identically, except only returns a static buffer instead of allocating the result each time. Appears at first look to have fit the bill nicely.

Now to hunt down the remaining little bastards who hide from me in the night and make fun of me.....

Post is unread #14 Oct 10, 2003 10:08 pm   
Go to the top of the page
Go to the bottom of the page

Samson
Black Hand
GroupAdministrators
Posts3,685
JoinedJan 1, 2002

 
After slogging through a few iterations of make/grind/fix leak/repeat, I've hit an impasse that is preventing me from reaching the next batch:

==14129== 30161 bytes in 803 blocks are still reachable in loss record 4 of 5
==14129== at 0x4002989A: calloc (vg_replace_malloc.c:273)
==14129== by 0x80FA594: str_alloc (hashstr.c:52)
==14129== by 0x80D7FF6: fread_string (db.c:618)
==14129== by 0x815B57B: load_class_file (tables.c:170)

Relevant section of load_class_file:

case 'N':
KEY( "Name", class->who_name, fread_string( fp ) );
break;

Now, since class->who_name should legitimately be getting filled like this, this shouldn't be a problem.

Later in the memory_cleanup ( handy function for leak checking btw ) we have:

/* Classes */
fprintf( stdout, "%s", "Classes.
" );
for( hash = 0; hash who_name );
DISPOSE( class );
}

Which, as you see, removed the class->who_name. Any idea why this one is tripping? It's keeping me from locating other problem areas that could benefit from using the new fread_flagstring

Pages:<< prev 1 next >>