Thread: Strcpy

  1. #1
    Registered User
    Join Date
    Nov 2007
    Posts
    21

    Question Strcpy

    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.

  2. #2
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    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
    Compilers can produce warnings - make the compiler programmers happy: Use them!
    Please don't PM me for help - and no, I don't do help over instant messengers.

  3. #3
    Registered User
    Join Date
    Nov 2007
    Posts
    21
    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.

    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);
    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?

  4. #4
    and the Hat of Guessing tabstop's Avatar
    Join Date
    Nov 2007
    Posts
    14,336
    Quote Originally Posted by Godders_2k View Post

    Code:
                    szPostStart = (char *)malloc(1000);
    	memset(szPostStart, 0, 999);
    	szPostStart ="For example";
    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.

  5. #5
    Registered User
    Join Date
    Nov 2007
    Posts
    21
    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.

  6. #6
    and the Hat of Guessing tabstop's Avatar
    Join Date
    Nov 2007
    Posts
    14,336
    Quote Originally Posted by Godders_2k View Post
    So there is absolutely no point in me doing the malloc in the first place?
    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.

    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?
    &szPostStart would be a pointer to pointer to char, while "For Example" would be a pointer to char, so that wouldn't work.

    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.

  7. #7
    and the hat of sweating
    Join Date
    Aug 2007
    Location
    Toronto, ON
    Posts
    3,545
    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:
    Code:
    szPostStart = (char *)malloc(1000);
    memset(szPostStart, 0, 999);
    Never cast malloc() (see the FAQ section for why).
    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().

  8. #8
    Deathray Engineer MacGyver's Avatar
    Join Date
    Mar 2007
    Posts
    3,210
    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).

  9. #9
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    Quote Originally Posted by cpjust View Post
    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:
    Code:
    szPostStart = (char *)malloc(1000);
    memset(szPostStart, 0, 999);
    Never cast malloc() (see the FAQ section for why).
    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().
    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].

    --
    Mats
    Compilers can produce warnings - make the compiler programmers happy: Use them!
    Please don't PM me for help - and no, I don't do help over instant messengers.

  10. #10
    and the hat of sweating
    Join Date
    Aug 2007
    Location
    Toronto, ON
    Posts
    3,545
    Quote Originally Posted by MacGyver View Post
    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).
    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.

  11. #11
    Deathray Engineer MacGyver's Avatar
    Join Date
    Mar 2007
    Posts
    3,210
    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.

  12. #12
    and the hat of sweating
    Join Date
    Aug 2007
    Location
    Toronto, ON
    Posts
    3,545
    Quote Originally Posted by MacGyver View Post
    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.

    http://www.cppreference.com/stdstring/strncpy.html
    If from has less than count characters, the remainder is padded with '\0' characters.

  13. #13
    Deathray Engineer MacGyver's Avatar
    Join Date
    Mar 2007
    Posts
    3,210
    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:

    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 &#37;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;
    }
    You can't get this program to write past the boundaries of szArgs, unless I made a simple mistake, which can be corrected.

    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.
    Last edited by MacGyver; 12-12-2007 at 11:57 AM.

  14. #14
    Officially An Architect brewbuck's Avatar
    Join Date
    Mar 2007
    Location
    Portland, OR
    Posts
    7,396
    Quote Originally Posted by cpjust View Post
    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.
    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:

    Code:
    strcpy(full_name, last_name);
    strcat(full_name, ", ");
    strcat(full_name, first_name);
    strcat(full_name, " ");
    strcat(full_name, middle_name);
    What if you wanted to make this safer by using the strncpy() and strncat() functions? You'd have to:

    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));
    And that's still not safe because strncpy() and strncat() may not place a null terminator in some cases! Compare with snprintf():

    Code:
    snprintf(full_name, sizeof(full_name), "&#37;s, %s %s", last_name, first_name, middle_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.

  15. #15
    Deathray Engineer MacGyver's Avatar
    Join Date
    Mar 2007
    Posts
    3,210
    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?

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. A Full Program to analyze.
    By sergioms in forum C Programming
    Replies: 2
    Last Post: 12-30-2008, 09:42 AM
  2. Using strcpy() and arrays of structs
    By vital101 in forum C Programming
    Replies: 3
    Last Post: 04-26-2007, 09:04 PM
  3. Where is strcpy() defined? (Can't find in string.h ect)
    By Zero_Point in forum C++ Programming
    Replies: 6
    Last Post: 04-03-2006, 05:14 PM
  4. Question about strcpy
    By Kevinmun in forum C Programming
    Replies: 4
    Last Post: 11-02-2005, 11:00 PM
  5. strcpy
    By Luigi in forum C++ Programming
    Replies: 17
    Last Post: 02-16-2003, 04:11 PM