How to interpret this debugging error

Question

As a hacker-in-training, I decided to go about making my own string_reverse function that takes a string, allocates memory for a new string, and returns a pointer to a new string, but I'm not getting what I desire, as this returns a segmentation fault.

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


char* string_reverse(char* string);

char* string_reverse(char* string) {
  int len = 0;
  for (int i = 0; *(string + i) != '\0'; ++i)
    len++;

  char* result = (char*)malloc(len * sizeof(char));
  if (result == NULL){
    puts("Pointer failure");
    exit(EXIT_FAILURE);
  }

  for (int i = 0; *(string + i) != '\0'; ++i)
    *(result + (len - i)) = *(string + i);

  return *result;
}

int main() {
  char* str= "Ni Hao!";
  char* result = string_reverse(str);

  printf("%s\n", result);
  free(result);
  return 0;
}

In return, I get this debugging message:

Starting program: /home/tmo/string_reverse 

Program received signal SIGSEGV, Segmentation fault.
0xb7e5b3b3 in strlen () from /lib/i686/cmov/libc.so.6

How should I interpret this result?

Answer 1

Your code didn't add the null terminator to the reversed string. As a result the printf function crashed trying to calculate it's length.

Change the malloc line to the following

char* result = (char*)malloc((len+1) * sizeof(char));

And you need to add the following line to the end of the string_reverse function in order to ensure the string has a null terminator.

result[len] = '\0';

Couple of other comments

• sizeof(char) is not needed. The size of the char is one of the few types defined by the C standard and it's value is 1.
• The first loop can be replaced by a simple call to strlen

EDIT

Two other issues. The line which actually does the character copy appears to be incorrect. I believe it should be (len - i - 1). Otherwise the initial character write will occur at (result + len) which is the place of the null terminator.

*(result + ((len - i) - 1)) = *(string + i);

Also, don't dereference result on return

Answer 2

Also you shouldn't be dereferencing result at the end of the function because its allready a pointer to your resulting string.

return result;