[Home] [Downloads] [Search] [Help/forum]

Gammon Forum

See www.mushclient.com/spam for dealing with forum spam. Please read the MUSHclient FAQ!

[Folder]  Entire forum
-> [Folder]  MUDs
. -> [Folder]  MUD Design Concepts
. . -> [Subject]  Custom codebase help
Home  |  Users  |  Search  |  FAQ
Username:
Register forum user name
Password:
Forgotten password?

Custom codebase help

It is now over 60 days since the last post. This thread is closed.     [Refresh] Refresh page


Posted by Zalethon   USA  (13 posts)  [Biography] bio
Date Sun 09 Dec 2007 03:46 PM (UTC)

Amended on Sun 09 Dec 2007 03:51 PM (UTC) by Zalethon

Message
Seems like kind of a dumb place to ask, but the only one on the forum that fits :P

I was working on a function to at least temporarily fix some MXP problems, and I got it working except that there's a memory leak in the function I'm not seeing. (This is for a custom codebase, not PennMUSH or anything)

char *smash_mxp ( char *str )
{
        int i, j = 0;
        char *str1;
        str1 = malloc(sizeof(str));
        memmove(str1,str,strlen(str)+1);

        for (i = 0; str1[i] != '\0';i++)
        {
                switch (str1[i])
                {
                        case '<':
                        str[j] = '&';
                        str[j +1] = 'l';
                        str[j +2] = 't';
                        str[j +3] = ';';
                        j +=3;
                        break;
                        case '>':
                        str[j] = '&';
                        str[j +1] = 'g';
                        str[j +2] = 't';
                        str[j +3] = ';';
                        j +=3;
                        break;
                        case '&':
                        str[j] = '&';
                        str[j +1] = 'a';
                        str[j +2] = 'm';
                        str[j +3] = 'p';
                        str[j +4] = ';';
                        j +=4;
                        break;
                        default:
                        str[j] = str1[i];
                        break;
                }
                j++;
        }
        free(str1);
        return str;
}


I'm aware there's probably a dozen better ways to go about doing this, but before we get into that I'd like to understand why the allocated memory isn't being freed.. Then, by all means :P

I'm also aware that that will break things for people whose clients don't support MXP, but it's only temporary, really. I could always check for support, if need be.

Overkill is better than underkill.
[Go to top] top

Posted by Nick Cash   USA  (626 posts)  [Biography] bio
Date Reply #1 on Sun 09 Dec 2007 04:22 PM (UTC)

Amended on Sun 09 Dec 2007 04:31 PM (UTC) by Nick Cash

Message
Well, I can see one problem.

        str1 = malloc(sizeof(str));


This is a bad allocation. You are allocating four bytes (the size of a pointer) to str1. I think your original intention was to allocate enough space to copy the string, which is somewhat different:


        str1 = malloc((strlen(str)+1)*sizeof(char));


Supposing the compiler will accept that... I'm so used to using C++'s string data type and new operator I forget my malloc syntax.

So if that code was actually working consider yourself lucky. A few other comments though. This function adds 4-5 characters per preexisting special character. You may very well need to resize your base string to accomadate this, as your function currently assumes it won't overrun its bounds. A good way to do this might be to scrap your incoming string and return your newly allocated one. This has other implications in the calling functions, however.

~Nick Cash
http://www.nick-cash.com
[Go to top] top

Posted by Zalethon   USA  (13 posts)  [Biography] bio
Date Reply #2 on Sun 09 Dec 2007 04:40 PM (UTC)
Message
Quote:
So if that code was actually working consider yourself lucky. A few other comments though. This function adds 4-5 characters per preexisting special character. You may very well need to resize your base string to accomadate this, as your function currently assumes it won't overrun its bounds. A good way to do this might be to scrap your incoming string and return your newly allocated one. This has other implications in the calling functions, however.


It actually was working. I had it adding the characters to str1 before, and returning str1, but it didn't work for some dumb reason. I might have just screwed it up, I'll try again in a bit...

Overkill is better than underkill.
[Go to top] top

Posted by Nick Gammon   Australia  (21,322 posts)  [Biography] bio   Forum Administrator
Date Reply #3 on Sun 09 Dec 2007 07:18 PM (UTC)
Message
Quote:

... but it didn't work for some dumb reason ...


Whiteknight explained the reason. You were making an existing string longer, which would have corrupted memory past the end of it.

He is also correct about changing the malloc to use strlen and not sizeof.

Even then, you need to malloc the new string to allow for the extra stuff you are adding. Take a look at the way I did it here:

http://www.gammon.com.au/mushclient/addingservermxp.htm

In that post, I first calculate how much extra memory is needed (by looking for <, >, and & characters). Then I allocate the correct extra amount of memory. Then I copy the text.

- Nick Gammon

www.gammon.com.au, www.mushclient.com
[Go to top] top

Posted by Nick Cash   USA  (626 posts)  [Biography] bio
Date Reply #4 on Tue 11 Dec 2007 04:48 PM (UTC)

Amended on Tue 11 Dec 2007 04:49 PM (UTC) by Nick Cash

Message
Quote:


str1 = malloc((strlen(str)+1)*sizeof(char));



I should note that the char data type is typically (I think guaranteed even) to be one byte, so you could rewrite the line to:


str1 = malloc(strlen(str)+1);


Which is a bit less confusing.

Of course you will, like Nick said, want to calculate how much extra memory you will need first.

~Nick Cash
http://www.nick-cash.com
[Go to top] top

The dates and times for posts above are shown in Universal Co-ordinated Time (UTC).

To show them in your local time you can join the forum, and then set the 'time correction' field in your profile to the number of hours difference between your location and UTC time.


5,327 views.

It is now over 60 days since the last post. This thread is closed.     [Refresh] Refresh page

Go to topic:           Search the forum


[Go to top] top

Quick links: MUSHclient. MUSHclient help. Forum shortcuts. Posting templates. Lua modules. Lua documentation.

Information and images on this site are licensed under the Creative Commons Attribution 3.0 Australia License unless stated otherwise.

[Home]


Written by Nick Gammon - 5K   profile for Nick Gammon on Stack Exchange, a network of free, community-driven Q&A sites   Marriage equality

Comments to: Gammon Software support
[RH click to get RSS URL] Forum RSS feed ( https://gammon.com.au/rss/forum.xml )

[Best viewed with any browser - 2K]    [Hosted at FutureQuest]