Printable View
Hi, I was just wondering if anyone could confirm for me exactly how the strcpy function works. Basically if I have a malloc'ed value and i strcpy it into a fixed size array, will the process of strcpy automatically free up the memory being used by the malloc'ed value as when I try to use the 'free' command on the value I get an error, almost as if it has already been freed when the strcpy function was used.
Cheers.
strcpy doesn't call any of malloc, free or related functions. If you get an error message when you free it, it's probably because you have overwritten the data that malloc keeps after the allocated block [or before?].
Did you remember to malloc 1 extra byte [for the zero end-marker] when you malloc the space for the string?
--
Mats
What I have is a fixed size array which is char szPostStatement[5000] where I have initialised the array to have NULLs for all the locations. I have also declared a char *szPostStart and a char *PostEnd at the top of my script. When it comes to use these values I do the following. SzClientIndex is another fixed size array which I captured at an earlier point.
It is the free's which are causing me a problem. If I take them out it works fine. I also do exactly the same process at a later part in my script, which had these values not been free'd, I would have thought would have spat up an error saying I was overwriting memory or such like. Any ideas?Code:szPostStart = (char *)malloc(1000);
memset(szPostStart, 0, 999);
szPostStart ="For example";
szPostEnd = (char *)malloc(1000);
memset(szPostEnd, 0, 999);
szPostEnd = "For example";
strcpy(szPostStatement, szPostStart);
strcat(szPostStatement, szClientIndexEncoded);
strcat(szPostStatement, szPostEnd);
free(szPostStart);
free(szPostEnd);
DO_Http(szPostStatement);
This is your leak here. You've allocated 1000 bytes of memory, but the last line sets the pointer szPostStart to point somewhere else (since it's pointing to a constant, it's not memory that's been malloc'ed, so it's not memory that can be free'd). You could strcpy here and not lose your malloc'ed memory.Quote:
Originally Posted by Godders_2k
So there is absolutely no point in me doing the malloc in the first place? Is what I should be doing setting it to NULL after I have used it? Or if I did &szPostStart = "For Example" would this force it to use the memory I had allocated for it and then subsequently allow me to free it? Sorry I'm still quite a novice at this stuff and learning on a daily basis.
That depends. Is szPostStart always going to be constant? The space for the string "For Example" is set aside ahead of time when the constant appears in the program, and you're stuck with it. If szPostStart is going to change all the time, and you need to read it in or whatever, then malloc() space for it. Now that you have this empty space, put your message in it. When you need to change szPostStart, overwrite the old stuff with the new stuff (it's your 1000 bytes now) and once you never need another Post, you can free() it.Quote:
Originally Posted by Godders_2k
&szPostStart would be a pointer to pointer to char, while "For Example" would be a pointer to char, so that wouldn't work.Quote:
Or if I did &szPostStart = "For Example" would this force it to use the memory I had allocated for it and then subsequently allow me to free it?
Executive summary: Use malloc() if you don't know ahead of time what szPostStart is going to be, and read your data into that memory. If szPostStart is going to be a constant, or one of a couple of constants, then you don't need to malloc(), just set szPostStart="For example" for example.
You should also avoid using strcpy(), strcat()... and use the 'n' versions instead (i.e. strncpy(), strncat()...), otherwise you could have a buffer overflow if you're not careful.
Also:
Never cast malloc() (see the FAQ section for why).Code:szPostStart = (char *)malloc(1000);
memset(szPostStart, 0, 999);
You're allocating 1000 chars but then you're only setting 999 of them to '\0'.
BTW, You can combine these two statements by calling calloc() instead of malloc().
strcpy() and strcat() are actually safe provided you use a dynamically sized buffer (or you take precautions that your static buffer is the proper size).
Or not use memset at all, as the string buffer doesn't have to be all zeros. strcpy() and such will set the final byte to zero [unless you overflow the buffer].Quote:
Originally Posted by cpjust
--
Mats
strcpy() & strcat() copy everything until they run into a '\0'. So if your src string is missing the terminating '\0' or if it's longer than the dest, you just wrote into memory you don't own.Quote:
Originally Posted by MacGyver
If you're missing '\0' chars on the ends of your strings, using the n version of string functions won't save you. ;)
You probably already screwed up your buffer along the way.
It won't fix the bug you have in your string, but it'll stop you from writing past the end of your array.Quote:
Originally Posted by MacGyver
http://www.cppreference.com/stdstring/strncpy.html
Quote:
If from has less than count characters, the remainder is padded with '\0' characters.
There's no possibility to overwrite your buffer if you use it right.
Here. Let's try something. Here's something I wrote quickly. Try to break it:
You can't get this program to write past the boundaries of szArgs, unless I made a simple mistake, which can be corrected.Code:#include <stdio.h>
#include <stdlib.h>
#include <string.h>
int main(int argc, char *argv[])
{
char *szArgs = NULL;
size_t i, len = 0;
for(i=0;i<(size_t)argc;i++)
{
len += strlen(argv[i]) + 1;
}
szArgs = malloc(len);
if(!szArgs)
{
fprintf(stderr, "Unable to allocate buffer of size %d\n", len);
return 1;
}
strcpy(szArgs, argv[0]);
for(i=1;i<(size_t)argc;i++)
{
strcat(szArgs, " ");
strcat(szArgs, argv[i]);
}
printf("szArgs = %s\n", szArgs);
printf("len = %d\n", len);
for(i = 0;i < len;i++)
{
printf("szArgs[%2d] =\t[%3d][%c]\n", i, szArgs[i], szArgs[i]);
}
free(szArgs);
return 0;
}
Of interest, while you are pushing the n versions of the string functions to be safer, I believe Salem mentioned before that the '\0' is left off when using strncpy() if your destination length is less than your source length. ;)
That means you have to add the '\0' yourself in those cases, which just opens the door to more issues.
If you are using sequences of strcpy() and strcat() to build up a string, it's better to just use snprintf(). Look at this mess:Quote:
Originally Posted by cpjust
What if you wanted to make this safer by using the strncpy() and strncat() functions? You'd have to:Code:strcpy(full_name, last_name);
strcat(full_name, ", ");
strcat(full_name, first_name);
strcat(full_name, " ");
strcat(full_name, middle_name);
And that's still not safe because strncpy() and strncat() may not place a null terminator in some cases! Compare with snprintf():Code:strncpy(full_name, last_name, sizeof(full_name));
strncat(full_name, ", ", sizeof(full_name) - strlen(full_name));
strncat(full_name, first_name, sizeof(full_name) - strlen(full_name));
strncat(full_name, " ", sizeof(full_name) - strlen(full_name));
strncat(full_name, middle_name, sizeof(full_name) - strlen(full_name));
snprintf() DOES guarantee a null terminator, it is more efficient (since it doesn't traverse the string over and over with each strcat() ) and it makes it clear what you are actually doing -- formatting a string.Code:snprintf(full_name, sizeof(full_name), "%s, %s %s", last_name, first_name, middle_name);
Again, I ask, why not just dynamically size your buffer for sprintf() or properly size the static buffers to eliminate the chance of a buffer overflow? Why run a risk of cutting off part of your message?
if the consequences of that happening are not severe it might be worth the simplicity.Quote:
Originally Posted by MacGyver
The key part here being "if you use it right".Quote:
Originally Posted by MacGyver
I think the n string functions make it harder (but obviously not impossible) to "use it wrong".
brewbuck:
Is snprintf() a standard C function? I've never heard of it and I don't see it listed on this site: http://www.cppreference.com/all_c_functions.html
Is it a C99 function?
Yes, it is C99. I so often forget.Quote:
Originally Posted by cpjust