Remove Trailing Spaces in C

Question

I'm trying to remove trailing spaces however I keep getting a segmentation fault. Not too sure where I am accessing memory out of bounds but this is my code. Leading spaces works fine.

String is the input for the function.

//remove leading spaces
char* final = string;
while(isspace((unsigned char)final[0]))
        final++;

 //removing trailing spaces
//getting segmentation fault here
    int length = strlen(final);
    while(length > 0 && isspace((unsigned char)final[length-1]))
            length--;
    final[length-1] = '\0';

The string I tested was

char* word = "  1 2 Hello    ";
printf("%s\n", removeSpaces(word));

When I comment out the trailing spaces, it works perfectly. I don't why the code is failing at the trailing spaces. I would really appreciate the help.

Possible duplicate of [How do I trim leading/trailing whitespace in a standard way?](https://stackoverflow.com/questions/122616/how-do-i-trim-leading-trailing-whitespace-in-a-standard-way) — Ian, Sep 29 '18 at 15:45
Your `word` points to a **readonly** array of char. You cannot change them, you need to make a copy into writable memory. To help themselves some people use `const char *word = " 1 2 hello ";` — pmg, Sep 29 '18 at 15:49
It works for leading spaces because you just increase the pointer, you don't change the contents of the (**readonly**) array. — pmg, Sep 29 '18 at 20:13

AlenL · Accepted Answer · 2018-09-29T16:15:16.157

1

The string literal " 1 2 Hello " is stored in read-only memory. Copy it first before attempting to write '\0' into it, and the problem will go away. So e.g., just replace this:

char* final = string;

with this:

char* final = strdup(string);

Edit 1: Upon considering this in more detail, I realized you also do a leading trim before trailing trim. Since you are moving the pointer, the allocation needs to happen after the leading trim, or the caller will not be able to free the memory. Here's a complete solution that shouldn't have any errors:

char *removeSpaces(const char *string) {

    while(isspace((unsigned char)string[0]))
        string++;
    char *final = strdup(string);
    int length = strlen(final);
    while(length > 0 && isspace((unsigned char)final[length-1]))
        length--;
    final[length-1] = '\0';
    return final;
}

Edit 2: While I would not recommend it in this case, it might be useful to be aware that if the variable was declared like this:

char word[] = "  1 2 Hello    ";

It would have been in writable memory, and the problem would also not exist. Thanks to pmg for the idea.

The reason why it is not a good approach is that you are expecting the callers to the function to always provide writable strings, and that you will modify them. A function that returns a duplicate is a much better approach in general.

(Don't forget to free() the result afterwards!)

edited Sep 29 '18 at 16:15

answered Sep 29 '18 at 15:50

AlenL

418
3
7

Or use an array rather than a pointer: `char word[] = " 1 2 hello ";` – pmg Sep 29 '18 at 15:53
I would wager that since OP has this as a function, it will be called from various other places. The function should really return a duplicate and be documented with the fact that the caller is responsible for freeing. – AlenL Sep 29 '18 at 15:55
@pmg But yes, for a newbie, it is useful to know that that is also a possibility. I will amend my answer - thanks for the idea. – AlenL Sep 29 '18 at 16:02
1) `final[length-1] = '\0';` is bad (UB) when `length == 0`. 2) `strlen(final)` returns a `size_t`. Better to use that here than`int length`. – chux - Reinstate Monica Sep 29 '18 at 18:13
I didn't declare the array like word[] is because my input will not be in that form. – treatyoself Sep 30 '18 at 17:02
@treatyoself, array and pointer are interchangeable in this context. you can assign a char[] to a char*. The difference is that if you assign the string literal to a pointer at initialization, you are literally pointing to the string literal (read-only memory on most OSes today, and usually shared between all instances of the same text content in the program); and if you assign to an array at initialization, then the compiler generates code that fills a new array (on the stack if it's local like here) with that content. – AlenL Oct 01 '18 at 04:54
Thank you all for helping out! – treatyoself Nov 23 '18 at 16:35

Remove Trailing Spaces in C

1 Answers1