Why free caused a Segmentation fault in my code?-CodePudding

I try to implement a simple program that returns a reversed copy of string. Everything went well except a Segmentation fault at the end!! It I comment the last free(), nothing looks wrong but I know it is not right.

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

void revStr(char *str, char *newStr, int *idx)
{
    if( *str != '\0')
    {
        *idx = *idx 1;
        revStr(str 1, newStr, idx);
        printf("char=%c int=%d\n", *str,*idx);
        newStr[*idx]=*str;
        *idx = *idx 1;
    }
    else if (*idx !=0)
    {
        printf("End Size=%d\n",*idx);
        newStr = (char*)malloc( sizeof(char) * (*idx) );
        *idx=0;
    }
}

int main(void) {
    int idx =0;
    char * in = "This is a test string.";
    char * out;

    revStr(in, out, &idx);

    printf("Out:%s\n", out);
    free(out);

    return 0;
}

char=. int=0
char=g int=1
char=n int=2
char=i int=3
char=r int=4
char=t int=5
char=s int=6
char=  int=7
char=t int=8
char=s int=9
char=e int=10
char=t int=11
char=  int=12
char=a int=13
char=  int=14
char=s int=15
char=i int=16
char=  int=17
char=s int=18
char=i int=19
char=h int=20
char=T int=21
Out:.gnirts tset a si sihT
Segmentation fault (core dumped)

I tried to figure out from couple of question but in vain. Can someone help??

CodePudding user response：

In your main() function you pass your revStr() function an argument out of type char *.

out is empty and points to nothing.

So when your revStr() function tries to append/change whatever out is pointing to (nothing in this case) with the line

newStr[*idx] = *str;

It leads to a segmentation fault.

Another problem is that as a result of out being empty, the call

free(out);

becomes dangerous and leads to undefined behaviour.

In order to fix this you can allocate memory for out with malloc().

size_t in_size = strlen(in);
char *out = malloc(in_size   1);

// allocate memory for string
// and  1 for NUL terminator

Also you should add a NUL terminator \0 to the end of out your string signify the end.

out[in_size] = '\0';

Note:

When allocating memory for out, it is best to do it inside main(). Calling malloc() multiple times inside a recursive function is probably not something you want.

CodePudding user response：

Thanks to @RetiredNinja for an useful reference. I figured out my problem and even found myself made more unaware mistakes. Since no one provides a correct and complete answer, I answer my question.

First of all, this is my personal practice on recursion. If you are looking for a good example on reversing string. This one is definitely NOT!!

First mistake. When I declared the out pointer, I thought it is a null pointer.

char * out;
............
............
free(out);

As matter of fact, it is NOT. It is initialized with platform-dependent default value. This explains why a free() call on it will cause segmentation fault. It is not dynamically allocated, therefore can not be freed.

Second mistake. According to the idea of "passed by (copy of) values". When I passed the pointer "out" as parameter "newStr", the pointer "newStr" holds same address value as "out". However the address value in "out" remained unchanged when I changed the value in "newStr" to a newly allocated memory address from malloc(). My code never utilized the dynamically allocated memory. The pointer to it lost after the recursive call creating it ended.

Third and unattempted mistake. Despite not utilizing the dynamically allocated memory, my code is still able to print out another string stored in another memory segmentation. This is because my platform(gcc on ubuntu18) doesn't enforce a strict prohibition on variables boundary. The declaration "char * out;" only allocated a memory space to hold a char and a pointer to it. My code accidentally accessed the memory space beyond the declared char. This is considered unsafe and may cause segmentation fault right away on other strict platforms.