Why the free in line2 makes memcpy do nothing?? Is

问题:

void str_cpy(char **des,char* src){
   if(*des || *des==src)
     free(*des);
   *des = (char*)malloc(sizeof(char)*(strlen(src)+1));
   if(!*des)
     return ;
   memcpy ( *des, src, (strlen(src)+1)*sizeof(char) );
 }

Why the free in the line2 makes memcpy do nothing?? Is the free before malloc is o.k??

回答1:

No!!! :-) It is not OK for you to free(src) before trying to use it! (In the case where *dest == src, line 2 of your code, namely free(*dest) is essentially the same as free(src) and then you use src which from that point on does not point to a properly allocated memory block.)

Trying to use freed memory leads to undefined behaviour. When something is undefined anything goes! Maybe free has changed the first byte in src to 0 (in which case strlen(src) == 0) or maybe memcpy detects the source memory is not properly allocated or maybe something else is going on. It is not really worth pursuing the question of why you get this behaviour. If you change your build settings (debug/optimise etc.) or build your program on a different system (actually it is the library, not the compiler that you use is what makes a difference here) you will probably get a different result.

At any rate, when *dest == src, you need not free, strlen, malloc, and copy anything; you can just return.

void str_cpy(char **des,char* src){
   if(*des==src) return;
   ...
 }

In general, someone using your function would expect that you do not do anything funny with the second argument, src, such as change its contents or, indeed, freeing its memory. I would also change the argument type to const char * src. While this does not stop you from freeing the src memory block, it will make the intended usage of the function clearer.

Actually, there is another, bigger problem with the function signature. *dest may be pointing to previously allocated memory in the heap (the only case in which free(*dest) is OK, assuming *dest != src, of course) or to previously allocated stack memory (in which case free(*dest) is not OK), but is also possible that it is pointing to statically allocated memory (a string literal) or to not be pointing to allocated memory period.

Standard library functions avoid this problem by not using such signatures (i.e. with arguments of type char **).

The signatures of the functions most relevant to what you are doing are:

 char * strcpy(char *restrict s1, const char *restrict s2);

 char * strncpy(char *restrict s1, const char *restrict s2, size_t n);

 char * strdup(const char *s1);

More info at: http://www.manpagez.com/man/3/strncpy/ and http://www.manpagez.com/man/3/strncpy/

None of these functions have to worry about where memory is allocated. All three require that their arguments are valid pointers (to either statically or dynamically allocated (heap/stack) memory. I suspect strdup is actually the most appropriate for what you are doing (you seem to want to allocate new memory for the duplicated string). strcpy and strncpy do not allocate new memory. (See the documentation links above for more details.)

Do you see a pattern there? strdup returns a pointer to the newly allocated memory (exactly as malloc does) and strcpy and strcpy take char * arguments and do not allocate memory. Neither of these functions have to deal with the problem your function has, of determining (impossible to do in C) what kind of memory *dest points to. There is a reason why Kernighan and Ritchie chose to do things that way :-)

You could provide documentation that makes it explicit that *dest must be pointing to a previously allocated memory block in the heap or be equall to NULL. However, good library designers would not do that because it passes responsibility to the users (as users will make mistakes).

Relying on documentation (i.e. having the library designer passing the responsibility to its users) is a treacherous path. It is best to avoid the problem altogether. Good C code starts at function signatures :-)

One last note: you are using strlen() twice in your code. For performance reasons, it is best if you only use it once and store its return value in a local variable and use that variable where you are currently invoking strlen().

回答2:

In C,You can only free the memory which is allocated by malloc , calloc or realloc. So freeing the memory other than that is Undefined behaviour

I your case it's not clear that what is *des. and whether it is allocated by the malloc , calloc or realloc.

Or you have done like this in calling enviroment

char des[20]= "hello , world \n";

or may be

char * des= "hello . world \n";

回答3:

As was pointed out to you in the comments, your function needs documentation. It works exactly as you want it to if the input is correct:

This works:

char * str1 = "hello";
char * str2 = malloc(6);   // Of course this is pointless since you'll free it anyway
str_cpy(&str2, str1);

So does this:

char * str1 = "hello";
char * str2 = NULL;
str_cpy(&str2, str1);

But this:

char * str1 = "hello";
char * str2;
str_cpy(&str2, str1);

Or this:

char * str1 = "hello";
char str2[6];
str_cpy(&str2, str1);

Or this:

char * str1 = "hello";
char * str2 = str1;
str_cpy(&str2, str1);

Will crash and burn. You need to note all the pre/post condidtions of your function in the comments.

回答4:

void str_cpy(char **des,char* src){
   if(*des || *des==src)
     free(*des);
   *des = (char*)malloc(sizeof(char)*(strlen(src)+1));
   if(!*des)
     return ;
   memcpy ( *des, src, (strlen(src)+1)*sizeof(char) );
 }

• Freeing before malloc is not necessary, it may contain some garbage value, and you are freeing that garbage which results (Core Dumped).
• If ( *des == src ) and you free des, which mean 'src' is also freed, and below you are using src in memcpy.

Correct Approach:

void str_cpy(char **des,char* src){
   *des = (char*)malloc(sizeof(char)*(strlen(src)+1));
   if(!*des)
     return ;
   memcpy ( *des, src, (strlen(src)+1)*sizeof(char) );
 }