Thread: Memcpy(), Structs, Free()

  1. #1
    Registered User
    Join Date
    Jun 2008
    Posts
    93

    Memcpy(), Structs, Free()

    I wrote the sample program below to demsonstrate a problem I am having. Basically, I have written a method that will create a copy of a struct that was created in main(). My goal is to free the memory that was used by both the original struct and the copy by the program's end. However, it crashes after returning from the copy method when I attempt to free the original structure. It is my understanding that if I do not free both I will have a memory leak situation. Does anyone know why the free()'s in main() crash this program?

    Code:
    #include<stdio.h>
    #include<stdlib.h>
    #include<string.h>
    
    #define STRINGA "abcdefghijklm"
    #define STRINGB "nopqrstuvwxyz"
    
    struct stuff
    {
    	char* a;
    	char* b;
    };
    
    int main()
    {
    	struct stuff *original = malloc(sizeof(struct stuff));
    	
    	original->a = malloc(strlen(STRINGA));
    	strcpy(original->a, STRINGA);
    
    	original->b = malloc(strlen(STRINGB));
    	strcpy(original->b, STRINGB);
    
    	printf("original->a: %s\n", original->a);
    	printf("original->b: %s\n", original->b);
    
    	copy((void*)original);
    
    	free(original->a);
    	free(original->b);
    	free(original);
    }
    
    void copy(void *ptr)
    {
    	printf("ptr->a: %s\n", ((struct stuff *)ptr)->a);
    	printf("ptr->b: %s\n", ((struct stuff *)ptr)->b);
    
    	struct stuff *new = malloc(sizeof(struct stuff));
    
    	memcpy(new, ptr, sizeof(struct stuff));
    
    	printf("new->a: %s\n", new->a);
    	printf("new->b: %s\n", new->b);
    
    	free(((struct stuff *)ptr)->a);
    	free(((struct stuff *)ptr)->b);
    	free((struct stuff *)ptr);
    }

  2. #2
    Registered User
    Join Date
    Jun 2008
    Posts
    93
    Sorry, this is the wrong sample. I'll post the correct one .... I know why this one crashes.

  3. #3
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    28,413
    The problem is that memcpy() copies the pointers, not what the pointers point to, hence the first two calls to free() in copy() eventually lead to a double free problem.
    Quote Originally Posted by Bjarne Stroustrup (2000-10-14)
    I get maybe two dozen requests for help with some sort of programming or design problem every day. Most have more sense than to send me hundreds of lines of code. If they do, I ask them to find the smallest example that exhibits the problem and send me that. Mostly, they then find the error themselves. "Finding the smallest program that demonstrates the error" is a powerful debugging tool.
    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  4. #4
    Registered User
    Join Date
    Jun 2008
    Posts
    93
    So freeing the original will also free the copy and vice versa?

  5. #5
    Registered User
    Join Date
    Jun 2008
    Posts
    93
    In order to make a true copy of the stuct I would have to memcpy ptr->a and ptr->b into the new structure in the copy method. Correct?

  6. #6
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    28,413
    Yes, i.e., you have to perform a deep copy.
    Quote Originally Posted by Bjarne Stroustrup (2000-10-14)
    I get maybe two dozen requests for help with some sort of programming or design problem every day. Most have more sense than to send me hundreds of lines of code. If they do, I ask them to find the smallest example that exhibits the problem and send me that. Mostly, they then find the error themselves. "Finding the smallest program that demonstrates the error" is a powerful debugging tool.
    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  7. #7
    Registered User
    Join Date
    Jun 2008
    Posts
    93
    That makes perfect sense. So the following example should not result in any memory leaks:

    Code:
    #include<stdio.h>
    #include<stdlib.h>
    #include<string.h>
    
    #define STRINGA "abcdefghijklm"
    #define STRINGB "nopqrstuvwxyz"
    
    struct stuff
    {
    	char* a;
    	char* b;
    };
    
    int main()
    {
    	struct stuff *original = malloc(sizeof(struct stuff));
    	
    	original->a = malloc(strlen(STRINGA));
    	strcpy(original->a, STRINGA);
    
    	original->b = malloc(strlen(STRINGB));
    	strcpy(original->b, STRINGB);
    
    	printf("original->a: %s\n", original->a);
    	printf("original->b: %s\n", original->b);
    
    	copy((void*)original);
    
    	printf("\nDone!\n\n");
    }
    
    void copy(void *ptr)
    {
    	printf("ptr->a: %s\n", ((struct stuff *)ptr)->a);
    	printf("ptr->b: %s\n", ((struct stuff *)ptr)->b);
    
    	struct stuff *new = malloc(sizeof(struct stuff));
    
    	memcpy(new, ptr, sizeof(struct stuff));
    
    	printf("new->a: %s\n", new->a);
    	printf("new->b: %s\n", new->b);
    
    	free(((struct stuff *)ptr)->a);
    	free(((struct stuff *)ptr)->b);
    	free((struct stuff *)ptr);
    
    	free(new);
    }

  8. #8
    Officially An Architect brewbuck's Avatar
    Join Date
    Mar 2007
    Location
    Portland, OR
    Posts
    7,396
    You're not allocating enough space for the strings. You need strlen(x) + 1 to account for the null terminator.
    Code:
    //try
    //{
    	if (a) do { f( b); } while(1);
    	else   do { f(!b); } while(1);
    //}

  9. #9
    C / C++
    Join Date
    Jan 2006
    Location
    The Netherlands
    Posts
    312
    Am I right you're not free()-ing new->a and new->b ?
    When I use

    Code:
    free(new->a);
    free(new->b);
    free(new);
    in the copy() function it works perfectly.
    Operating Systems:
    - Ubuntu 9.04
    - XP

    Compiler: gcc

  10. #10
    Registered User
    Join Date
    Jun 2008
    Posts
    93
    When you free original->a and original->b, new->a and new->b are also being freed because the a and b pointers point to the same memory in both structs. When you do a memcpy on a struct it only copies the memory allocated for the struct and not the memory that fields in the struct may point to. If I were to also do a memcpy on original->a and original->b into new->a and new->b then I would have to free these fields. So calling free on the a and b in the original or in the new will free these fields the way it is now. If you attempt to free both fields in both structs you will encounter a double free or corruption error.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Structs and mem allocation
    By reversaflex in forum C Programming
    Replies: 2
    Last Post: 07-03-2008, 07:46 AM
  2. free() doesn't seem to work...
    By AlienJedi in forum C Programming
    Replies: 10
    Last Post: 01-29-2008, 05:27 PM
  3. Free Store of memory
    By George2 in forum C++ Programming
    Replies: 6
    Last Post: 11-12-2007, 02:27 PM