anything wrong with this trim() method in C

Question

this method is for trimming a string in C by deleting spaces from the beginning and end. BTW iam not that good with C and actually facing some issues with dealding with strings

char* trim(char s[])
{
    int i, j;
    int size = strlen(s);

    //index i will point to first char, while j will point to the last char
    for(i = 0; s[i] == ' '; i++);
    for(j = size - 1; s[j] == ' '; j--);

    if(i > 0)
        s[i - 1] = '\0';

    if(j < size - 1)
        s[j + 1] = '\0';

    s = &s[i];

    return s;
}

Couple things to think of: `char *res = trim(" literal ");` or `char spaces[] = " "; char *res = trim(spaces);` — pmg, May 06 '22 at 16:24
what is the difference between for example: char s[20] and char *s? — Mahmoud2002, May 06 '22 at 16:30
yes it is different. [refer](https://stackoverflow.com/questions/1704407/what-is-the-difference-between-char-s-and-char-s). If you are passing `char *s = " hello "` as argument then it is UB. — TruthSeeker, May 06 '22 at 16:32
For difference between array and pointer, you might like section 6 of the [comp.lang.c faq](http://c-faq.com/). — pmg, May 06 '22 at 16:33
ok i just looked it up and the difference is that char[20] is statically allocated but char* is dynamic. also is there any improvment could be made on my implementation of trim()? — Mahmoud2002, May 06 '22 at 16:35
"is there any improvment..." before thinking of improvement make sure the implementation is correct. It's a waste of time and effort "improving" a wrong implementation. — pmg, May 06 '22 at 16:39
`char*` doesn't mean a dynamically allocated. It means a pointer to `char`. `char *s = " hello "` here it is pointing to string literals. [The Definitive C Book Guide and List](https://stackoverflow.com/q/562303/4139593) would help you understand language basics. — TruthSeeker, May 06 '22 at 16:40
See https://stackoverflow.com/questions/164194/why-do-i-get-a-segmentation-fault-when-writing-to-a-string-initialized-with-cha — Barmar, May 06 '22 at 16:47
The `char spaces[] = " ";` example also applies to empty strings: `char empty[] = ""; char *res = trim(empty);`. — Ian Abbott, May 06 '22 at 17:02
Changing `for(j = size - 1; s[j] == ' '; j--);` to `if (s[i])` `for(j = size - 1; s[j] == ' '; j--);` should take care of all-spaces and empty strings. Then you just need to ensure that you never pass a string literal to the function. — Ian Abbott, May 06 '22 at 17:05

score 0 · Answer 1 · answered May 06 '22 at 17:19

This loop

for(j = size - 1; s[j] == ' '; j--);

will access an out-of-bounds index when:

the input string consists entirely of spaces (e.g., " "), in which case there is nothing stopping j from reaching -1 and beyond, or
the input string is the empty string (""), where j starts at -1.

You need to guard against this in some way

for (j = size - 1; j >= 0 && s[j] == ' '; j--)

The other thing to consider is that you are both: modifying the contents of the original string buffer, and potentially returning an address that does not match the beginning of the buffer.

This is somewhat awkward, as the user must take care to keep the original value of s, as its memory contains the trimmed string (and might be the base pointer for allocated memory), but can only safely access the trimmed string from the returned pointer.

Some things to consider:

moving the trimmed string to the start of the original buffer.
returning a new, dynamically allocated string.

Some people have warned that you cannot pass a string literal to this function, which is true, but passing a string literal to a non-const argument is a terrible idea, in general.

anything wrong with this trim() method in C

1 Answers1