CODEWITHSUNDEEP
cfunctionsegmentation-faultreturnwarnings
rooxgray
rooxgray

Reputation: 13

Segmentation fault while implementing reverse function in C

I decided to make a reverse(s) function in C and in the process, I encountered 2 warning messages during compilation.

alexander@debian:~/Dropbox/GitRepo/M2MPL/text$ make reverse
cc     reverse.c   -o reverse
reverse.c: In function 'reverse':
reverse.c:29:2: warning: return makes integer from pointer without a cast [enabled by default]
reverse.c:29:2: warning: function returns address of local variable [enabled by default]
alexander@debian:~/Dropbox/GitRepo/M2MPL/text$

Since they were warnings I tried to ignore them and run the program, when I input an array of characters Dropbox using scanf("%s", str1);, the result appeared as Segmentation Fault.

alexander@debian:~/Dropbox/GitRepo/M2MPL/text$ ./reverse
Enter a string to reverse: Dropbox
Segmentation fault
alexander@debian:~/Dropbox/GitRepo/M2MPL/text$

Here's my implementation of the reverse(s) function:

/* reverse: returns a reversed string of s */
char reverse(char s[])
{
    int i, len=strlen(s);
    char result[LIMIT];

    for (i = len; i >= 0; --i) {
            result[len-i] = s[i];
    }

        return result;
}

FULL CODE HERE!

I need help to know why I got a Segmentation fault instead of xobporD as a result and if possible, a suggestion to fix this problem.

Upvotes: 1

Views: 117

Answers (3)

Mahonri Moriancumer
Mahonri Moriancumer

Reputation: 6003

There are a few flaws in the question code.

  • It is likely that the return type should be 'char *' rather than 'char'.
  • Attempting to return local stack memory to the caller.
  • The result string is likely missing a string termination character.

The following code fix the above. It returns the string in allocated 'heap' memory to the caller. (The caller should free() the memory when it is no longer needed.):

/* reverse: returns a reversed string of s */
char *reverse(char *s)
   {
   int i, len=strlen(s);
   char *result = malloc(len+1);
   if(result)
      {
      result[len] = '\0';
      for(i = len; i >= 0; --i)
         result[len-i] = s[i];
      }

   return(result);
   }

Upvotes: 0

vlad_tepesch
vlad_tepesch

Reputation: 6883

First of all:
The compiler does not warn you just for fun. You shouldn't be surprised that your program crashes if you ignore compiler warnings.

You return the pointer to an automatic variable that goes out of scope at the end of the function. The pointer returned to the caller is not valid since it points to an object that no longer exists after the reverse function returns.

Another problem is that the first character that you copy is the '\0' so your resulting string is empty. You need to reverse the characters of the string, but still put the '\0' at the end of the string.

Upvotes: 2

Blagovest Buyukliev
Blagovest Buyukliev

Reputation: 43538

The first warning means that the return type indicated by the signature of the reverse function does not match the type of the variable that you are actually returning. The expected type is char, and you are returning a char * (because result decays to such a pointer). So the compiler transforms the pointer into an integer (char), which is usually not what's intended by the programmer.

The second problem is that you are returning the address of a local variable, and that variable is no longer available after the function returns, because of the way automatic storage works. If you want to return a pointer to some data, it must be allocated either globally (declared static inside the function or declared globally), or it must be dynamically allocated.

Upvotes: 2

Related Questions