Segmentation fault when detabing string

Question

I have a function to replace tabs with spaces in a string that looks like this:

#include <stdio.h>

char *detab(char *string) 
{
    for (int i = 0; string[i] != '\0'; i++)
        if (string[i] == '\t')
            string[i] = ' ';
    return string;
}

int main(int argc, char **argv)
{
    char *string = "\thello\thello";
    detab(string);
    printf("%s\n", string);
    return 0;
}

But when I run it on "\thello\thello\t", it produces a segmentation fault. Why does it do this? I'm pretty new to C, so I may be missing something trivial.

Answer 1

This could be because the calling code did not allocate enough space for the string. It must always allocate at least one space larger than the visible characters in the string to allow space for the \0.

That being said, since strings are mutable, there is no need to return the string. It it will modify the string as you are working.

Here would be a working version of your code:

void detab(char * myStr) 
{
    for (int i = 0; myStr[i] != '\0'; i++)
        if (myStr[i] == '\t')
            myStr[i] = ' ';
}

char theString[] = "\thello\thello\t";
printf("Before: %s", theString);
detab(theString);
printf("After: %s", theString);

Also, keep in mind the following:

char buffer[4] = "test"; //THIS IS NOT SAFE. It might work, but it will overwrite stuff it shouldn't
char buffer[5] = "test"; //This is Ok, but could be an issue if you change the length of the string being assigned.
char buffer[] = "test"; //This is preferred for string literals because if you change the size of the literal, it will automatically fit.

UPDATE: Base on the main method you added, here is your issue:

You need to change

char * string = "\thello\thello";

To

char string[] = "\thello\thello";

The reason is because, when you define a string literal and assign it to a char *, it resides in the text portion of memory, and cannot be modified safely. Instead, you should assign the string literal to a char [] (which can be passed as a char * since that is what its actual type is). This syntax will let the compiler know it should allocate space on the stack and populate it with the values in the string literal, allowing it to be modified.

char * joe = "blah" simply creates the char * pointer, and points it to the data in the text portion (which is immutable).

char joe[] = "blah" tells the compiler to create an array of the appropriate length on the stack, to load it with the string literal, to create the char * pointer, and then to point the pointer at the start of the array of data on the stack.

Answer 2

This works:

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

char *detab(char *string) 
{
    for (int i = 0; string[i] != '\0'; i++)
        if (string[i] == '\t')
            string[i] = ' ';
    return string;
}

int main ( int argc, char ** argv ) {
    char str[21] = "\thello\thello\t";

    printf( "%s\n", detab( str ) );

    return 0;
}

As others have said, it's probably segfaulting because you're modifying a string literal. With char str[21], the string literal is copied into the stack-allocated str, where it can then be modified by your function.

Answer 3

Are you sure the strings are always null terminated.

Try some belt and braces...

char *detab(char *string) 
{
    int s_len= strlen(string) + 1;

    for (int i = 0; string[i] != '\0'; i++)
    {
        if (string[i] == '\t')
        {    string[i] = ' '; }
        if (i == s_len) { /* failure - print some diagnostic */ return NULL; }
    }
    return string;
  }