Function that returns string is not working- segmentation fault

Question

I have a very very easy function that is giving me some problems. All I want to do is return a substring after changing it from a string. But when I run the code I have segmentation fault. What is happening with my code:

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

const char *new_name(char str[])
{

  char * pch;
  char * last="";
  char * pch2;
  static char name[80];
  printf ("Splitting string \"%s\" into tokens:\n",str);

  pch = strtok (str,"/");
  while (pch != NULL)
  {
     last=pch;
     pch = strtok (NULL, "/");

  }

  pch2 = strtok (last,".");
  strcpy(name, pch2);
  strcat(name, ".ppm");

  return name; 
}

int main()
{
  printf("New name: %s",new_name("/home/test/segmentation/test.pgm"));
  return 0;

}

EDIT: I have another question: I want to use the return of this function as the input for another function, but it accepts char and the value returned is const char. How to make the conversion?

Answer 1

strtok will modify its first parameter, so you can't pass a pointer to a string literal to it, which would invoke undefined behavior.

Change the first two lines of main to this:

char str[] = "/home/test/segmentation/test.pgm";
printf("New name: %s\n",new_name(str));

Answer 2

The crash is in strtok. The problem is you pass a pointer to a string literal as its first parameter, and string literals are usually in a read only memory, so strtok crashes when trying to change the string.

char *strtok(char *str, const char *delim);

One way to fix it:

char fileName[] = "/home/test/segmentation/test.pgm";
printf("New name: %s",new_name(fileName));

EDIT: Answer to your question below 'EDIT'.

Just change the signature of your function to return char *:

char *new_name(char str[])

However, it would be nice to change it a bit more. Instead of using the local static buffer (name) it would be better to pass the buffer as a second parameter, and allocate it before calling the function, in the client code. The problem with this approach is that usually the caller doesn't know the exact length or even maximum length of the string to be produced.

Another, perhaps better, option is to allocate the buffer's memory to be returned dynamically using malloc and return pointer to it to the caller. In this case the responsibility for releasing the buffer will be with the user code, so you should document it properly.

Answer 3

You have to remember that the strtok function modifies the string input. And you pass it a pointer to a literal string which is constant (i.e. read-only). Trying to modify a literal string leads to undefined behavior.

The solution is quite simple:

char str[] = "...";
printf("New name: %s\n", new_name(str));

Answer 4

strtok works by mutating the string it is passed. You cannot pass a constant string to strtok.

You can try

char str[] = "/home/test/segmentation/test.pgm";
printf("New name: %s",new_name(str));

instead.

Answer 5

new_name("/home/test/segmentation/test.pgm")

You are passing a string literal (read only), and strtok is modifying this string

Answer 6

The first parameter to strtok can not be a constant string(which is what a string literal is):

pch = strtok (str,"/");
              ^^^

since it modifies its first parameter, the linked document says:

char *strtok( char *restrict str, const char *restrict delim );

This function is destructive: it writes the '\0' characters in the elements of the string str. In particular, a string literal cannot be used as the first argument of strtok.

using a non const array will fix this problem:

char arr[]  = "/home/test/segmentation/test.pgm" ;
printf("New name: %s",newname(arr));

For completeness sake, modifying a string literal is undefined behavior the draft C99 standard in section 6.4.5 String literals paragraph 6 says (emphasis mine):

It is unspecified whether these arrays are distinct provided their elements have the appropriate values. If the program attempts to modify such an array, the behavior is undefined.