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;
}

Edward · Accepted Answer · 2017-02-09 17:52:36Z

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 `#include`s

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;
}

thank you for your review. learned a lot of new things with it :) — Matthias Burger
– Matthias Burger, Commented Feb 9, 2017 at 21:24

Stack Exchange Network

malloc and free of a char-array

1 Answer 1

Fix the bug

Use the required `#include`s

Use `const` where practical

Check for `NULL` pointers

Learn to use pointers instead of indexing

You must log in to answer this question.

Hot Network Questions

malloc and free of a char-array

1 Answer 1

Fix the bug

Use the required #includes

Use const where practical

Check for NULL pointers

Learn to use pointers instead of indexing

You must log in to answer this question.

Related

Hot Network Questions

Use the required `#include`s

Use `const` where practical

Check for `NULL` pointers