malloc and free of a char-array

Question

I wrote a function to reverse a char-array (string). Since I'm beginner and didn't work with malloc and stuff before, maybe someone could take a look, if this is fine, what I'm doing here?

char* reverse_string(char* string)
{
 // getting actual length of the string
 size_t length = strlen(string);
 // allocate some space for the new string
 char* new_string = malloc(sizeof(char)*(length+1));
 // index for looping over the string
 int actual_index = 0;
 // iterating over the string until '0円'
 while(string[actual_index] != '0円') 
 new_string[length-actual_index-1] = string[actual_index++];
 // setting the last element of string-array to '0円' 
 new_string[length] = '0円';
 // free up the allocated memory
 free(new_string);
 // return the new string
 return new_string;
}

Answer 1

Here are some things that may help you improve your code.

Fix the bug

Once memory is freed, it should not be referenced again. Unfortunately, your code allocates memory and then frees it and then returns a pointer to the freed memory. That's a serious bug! To fix it, simply omit the free within the function and make sure the caller calls free instead. Alternatively, you could avoid all of that by reversing the passed string in place.

Use the required #includes

The code uses strlen which means that it should #include <string.h> and malloc and free which means that it should #include <stdlib.h>. It was not difficult to infer, but it helps reviewers if the code is complete.

Use const where practical

In your revere_string routine, the string passed into the function is not and should not be altered. You should indicate that fact by declaring it like this:

char* reverse_string(const char* string)

Check for NULL pointers

The code must avoid dereferencing a NULL pointer if the call to malloc fails. The only indication that it has failed is if malloc returns NULL; if it does, it would probably make most sense to immediately return that NULL pointer.

Learn to use pointers instead of indexing

Using pointers effectively is an important C programming skill. This code could be made much simpler by doing an in-place reversal of the passed string and by using pointers:

char* reverse_string(char* string) {
 if (string == NULL) 
 return string;
 char *fwd = string;
 char *rev = &string[strlen(string)-1];
 while (rev > fwd) {
 char tmp = *rev;
 *rev-- = *fwd;
 *fwd++ = tmp;
 }
 return string;
}

• Matthias Burger: thank you for your review. learned a lot of new things with it :)