Register forum user name Search FAQ

Gammon Forum

Notice: Any messages purporting to come from this site telling you that your password has expired, or that you need to verify your details, confirm your email, resolve issues, making threats, or asking for money, are spam. We do not email users with any such messages. If you have lost your password you can obtain a new one by using the password reset link.
 Entire forum ➜ MUDs ➜ MUD Design Concepts ➜ Custom codebase help

Custom codebase help

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


Posted by Zalethon   USA  (13 posts)  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.
Top

Posted by Nick Cash   USA  (626 posts)  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
Top

Posted by Zalethon   USA  (13 posts)  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.
Top

Posted by Nick Gammon   Australia  (23,102 posts)  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
Top

Posted by Nick Cash   USA  (626 posts)  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
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.


19,167 views.

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

Go to topic:           Search the forum


[Go to top] top

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