Notice: Any messages purporting to come from this site telling you that your password has expired, or that you need to "verify" your details, 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
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,070 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.
18,974 views.
It is now over 60 days since the last post. This thread is closed.
Refresh page
![[Go to top]](/images/gototop.gif)
top