patterncMinor
Trim function in C
Viewed 0 times
trimfunctionstackoverflow
Problem
I am trying to write an idiomatic trim function in C. How does this look? Should I instead malloc the new string and return it?
void trim(const char *input, char *result)
{
int i, j = 0;
for (i = 0; input[i] != '\0'; i++) {
if (!isspace(input[i])) {
result[j++] = input[i];
}
}
}Solution
As @JeffMercado pointed out, this removes spaces instead of trimming leading and trailing spaces. Assuming you want to keep the current functionality, let's call it
There's a really subtle bug here:
Another bug: you don't emit a NUL terminator, meaning the caller would have no way of knowing how long the string is (unless it zeroed out the buffer before calling your function).
Fixing these bugs gives us:
@JeffMercado also said this function is vulnerable to buffer overflow. In a sense, this is not true, provided the caller knows to allocate a buffer of at least
See if you can implement this. Some things to bear in mind:
-
Don't forget about the NUL terminator when checking the output buffer size.
-
Don't be like strncpy and omit the NUL terminator when you have to truncate the string, as it can lead to subtle bugs.
-
If you use
remove_spaces.There's a really subtle bug here:
... isspace(input[i]) ...isspace takes the value of an unsigned char or EOF. Passing it a char, which is usually signed, will produce undefined behavior. Instead, say:... isspace((unsigned char) input[i]) ...Another bug: you don't emit a NUL terminator, meaning the caller would have no way of knowing how long the string is (unless it zeroed out the buffer before calling your function).
Fixing these bugs gives us:
void remove_spaces(const char *input, char *result)
{
int i, j = 0;
for (i = 0; input[i] != '\0'; i++) {
if (!isspace((unsigned char) input[i])) {
result[j++] = input[i];
}
}
result[j] = '\0';
}@JeffMercado also said this function is vulnerable to buffer overflow. In a sense, this is not true, provided the caller knows to allocate a buffer of at least
strlen(input) + 1. But the caller might be lazy and just say char result[100]. Adding an output buffer size parameter will likely guard against such a mistake:void remove_spaces(const char *input, char *output, size_t output_size);See if you can implement this. Some things to bear in mind:
-
Don't forget about the NUL terminator when checking the output buffer size.
-
Don't be like strncpy and omit the NUL terminator when you have to truncate the string, as it can lead to subtle bugs.
-
If you use
int for i and j and size_t for output_size, you should get compiler warnings about comparison between signed and unsigned. If you don't, turn up your compiler warnings. If you're using GCC from the command line, get in the habit of typing gcc -Wall -W.Code Snippets
... isspace(input[i]) ...... isspace((unsigned char) input[i]) ...void remove_spaces(const char *input, char *result)
{
int i, j = 0;
for (i = 0; input[i] != '\0'; i++) {
if (!isspace((unsigned char) input[i])) {
result[j++] = input[i];
}
}
result[j] = '\0';
}void remove_spaces(const char *input, char *output, size_t output_size);Context
StackExchange Code Review Q#20897, answer score: 4
Revisions (0)
No revisions yet.