Using free() function

Could someone explain what is the most correct usage of free() function in this paricular case:

Code:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>


void reverseStr(const char **str)
    {
    int i, j;
    int word_len = strlen(*str);

    char *tmp_word = (char *)malloc(word_len + 1);

    if(tmp_word == NULL) return;

    for(i=0, j=(word_len - 1); j>=0; i++, j--)
        {
        tmp_word[i] = (*str)[j];
        }

    tmp_word[word_len] = '\0';
    *str = tmp_word;

    // Putting free(tmp_word) here seems to erase the content of my_str
    // in main() function

    return;
    }


int main(void)
    {
    const char *my_str = "This is nice sentence.";

    printf("%s\n", my_str);
    reverseStr(&my_str);
    printf("%s\n", my_str); // now the string is reversed

    return 0;
    }

Code:

*str = tmp_word;

This is not valid syntax for copying a string in 'C'. Try "strcpy()" instead.

You should only free the memory you allocate when you're all done with it. In your case, that would be after the 2nd printf, on line 37. Calling free doesn't erase the contents, but it does free up that memory to be used by something else, which may overwrite the old data, giving the effect of erasing it. Also remember, calling free doesn't change the pointer, so my_str still points to the same bit of memory, but it now contains invalid data, and trying to access it results in undefined behavior.

Some people suggest that you allocate and free strings in the same context/function, to better keep track of memory usage and reduce the likelihood of memory leaks, but that's not always possible/practical, so diligence on your part is necessary.

You should not need to use free in that program because you should not need to use malloc in that program. How about trying to reverse the string in-place?

You will need to change your main function though because you can't reverse a string literal, you need to declare an array instead, and don't make it const.

i think the free function function is probably best for linked lists and in this case i think to free the string would be to make the string NULL..

Originally Posted by Nyah Check

i think the free function function is probably best for linked lists and in this case i think to free the string would be to make the string NULL..

No, you need free() for every memory block you have allocated in your program. Setting the string to NULL doesn't free the allocated memory block.

Bye, Andreas

Usually it is simpler to just return a new dynamically allocated copy:

Code:

#include <stdlib.h>
#include <string.h>
#include <errno.h>
#include <stdio.h>

char *reverse(const char *const string)
{
    char  *rev;
    size_t len, i;

    /* We do not accept NULL. */
    if (!string) {
        errno = EINVAL;
        return NULL;
    }

    /* Find out the length of the string, */
    len = strlen(string);

    /* and allocate memory for a copy. Remember: add '\0' at end! */
    rev = malloc(len + 1);
    if (!rev) {
        errno = ENOMEM;
        return NULL;
    }

    /* Copy the string contents in reverse order, */
    for (i = 0; i < len; i++)
        rev[i] = string[len - 1 - i];

    /* and add the end-of-string byte. */
    rev[len] = '\0';

    /* Return the dynamically allocated string. */
    return rev;
}

int main(int argc, char *argv[])
{
    int arg;
    char *reversed;

    for (arg = 1; arg < argc; arg++) {

        reversed = reverse(argv[arg]);
        if (!reversed) {
            fprintf(stderr, "%s: %s.\n", argv[arg], strerror(errno));
            return 1;
        }

        printf("%s reversed is %s.\n", argv[arg], reversed);

        free(reversed);
    }

    return 0;
}

The other approach is to let the user specify a (dynamically allocated) buffer, and the size of that buffer. New input routines in POSIX.2008 (#define _POSIX_C_SOURCE 200809L), getline() and getdelim() use this interface:

Code:

#include <stdlib.h>
#include <string.h>
#include <errno.h>
#include <stdio.h>

/* Returns a copy of the string, reversed, in *dataptr.
 * Returns the length of the string, or (ssize_t)-1 with errno set if error.
*/
ssize_t reverse(char **const dataptr, size_t *const sizeptr, const char *const source)
{
    char *temp;
    size_t length, i;

    /* NULL pointers are not accepted! */
    if (!dataptr || !sizeptr || !source) {
        errno = EINVAL;
        return (ssize_t)-1;
    }

    length = strlen(source);

    /* Do we need a larger data buffer? */
    if (*sizeptr <= length) {
        /* length + 1, or anything bigger suffices.
         * Experience shows that padding to some next multiple works better.
         * This adjusts *sizeptr to next multiple of 32, adding at least one. */
        *sizeptr = (length | 31) + 1;

        temp = realloc(*dataptr, *sizeptr);
        if (!temp) {
            free(*dataptr); /* free(NULL) is perfectly okay. */
            *sizeptr = 0;
            errno = ENOMEM;
            return (ssize_t)-1;
        }
        *dataptr = temp;
    }

    /* Copy routine. Note the downwards pointer arithmetic! */
    temp = *dataptr + length - 1;

    /* Set the end of string, temp[0] being the last character, */
    temp[1] = '\0';

    /* and copy the rest of the string in reverse. */
    for (i = 0; i < length; i++)
        temp[-i] = source[i];

    return (ssize_t)length;
}

int main(int argc, char *argv[])
{
    char *buffer = NULL; /* You must initialize this to NULL! */
    size_t allocated = 0; /* You must initialized this to zero! */
    ssize_t length;
    int arg;

    for (arg = 1; arg < argc; arg++) {

        length = reverse(&buffer, &allocated, argv[arg]);
        if (length == (ssize_t)-1) {
            fprintf(stderr, "%s: %s.\n", argv[arg], strerror(errno));
            return 1;
        }

        printf("%s (%d bytes): %s (%d bytes)\n", argv[arg], (int)strlen(argv[arg]), buffer, (int)length);
    }

    free(buffer);
    buffer = NULL;
    allocated = 0;

    return 0;
}

If you look carefully, the first implementation is simpler, but uses a lot of allocations. The second implementation is a bit more complex to use, but it dynamically grows the buffer to long enough but does not use extra allocations. If you call the function often, and reuse the same buffer (like in the example main() above), this usually gives a measurable speedup.

@Andipersti thanks for the tip...