You Have Several Buffer Overrun Bugs
If we pass in a pointer that isn’t properly terminated, this function will gleefully copy past the end of the input buffer, copying whatever is after the string into the new buffer. This could both cause a security bug (similar to Heartbleed) and potentially waste gigabytes of memory.
Always, always, always check for buffer overruns in C! It is never too soon to learn good habits!
Sizes are size_t
, not int
On most 64-bit systems, a size_t
is unsigned and 64-bits wide, and an int
is signed and 32-bits wide. There are a huge number of bugs you can cause by converting a size_t
to an int
or vice versa. If your compiler does not warn you about this line:
int input_length = strlen(input_copy);
you need to enable more warnings. On GCC, Clang or ICX, I typically use -Wall -Wextra -Wpedantic -Wconversion -Wdeprecated
and a -std=
option. (It’s -Wconversion
that should flag this.) On other compilers, check the documentation. Ideally, turn off warnings for headers from other projects, and compile with -Werror
as well.
Check the Buffer Size
It’s good practice in C to pass in the maximum size of the input buffer, so that, even if the input array is unterminated, the function cannot overrun its buffer.
Return a String Slice
I agree with the other answers that you should be using a different API. I disagree that you should be making a deep copy of the string at all. In this case, you can represent the trimmed string as a slice of the original. This is both safer and more optimized.
You can still easily make a deep copy of the string slice if you need to—but you often won’t need to. And your program will run much faster and use less memory if you can avoid it.
So let’s say we define this data structure:
// A const string slice:
typedef struct str_cslice_t {
const char* s;
size_t n;
} str_cslice_t;
Here’s some code demonstrating two ways we could output a slice without making a copy:
#include <assert.h>
#include <limits.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
// A const string slice:
typedef struct str_cslice_t {
const char* s;
size_t n;
} str_cslice_t;
#define BUF_SIZE 1024U
// Not re-entrant:
int main(void)
{
static char buffer[BUF_SIZE] = " foo bar ";
const str_cslice_t slice = { &buffer[3], 7 };
assert( slice.n <= INT_MAX);
printf( "\"%.*s\"\n", (int)slice.n, slice.s );
memmove( buffer, slice.s, slice.n ); // Invalidates slice.s!
buffer[slice.n] = '\0';
printf( "\"%s\"\n", buffer );
return EXIT_SUCCESS;
}
Most good C APIs (and also the ANSI C standard library) let you pass in either the length or the maximum length of your string. This is primarily to make it at least theoretically possible to write C code without memory bugs, but it’s often faster too. Here, I used the %.*s
format specifier to pass in the maximum length of the string, which lets us print a string slice. (A dangerous piece of technical debt in this interface is that the maximum length is passed as the “precision,” which has type int
. Just on principle, I add an assertion that this cast will not overflow. The compiler can optimize it out at runtime. An alternative would be to use fwrite
.)
After that, I re-used the original buffer by moving the slice to the beginning and then truncating it. You can re-use your input buffers this same way any time you no longer need the untrimmed string. That’s a lot of the time.
You can do most other things you could do with a deep copy of the string, using a string slice, as well. It’s normally faster and uses less memory, too! For example, you can concatenate slices with memcpy
rather than strings with strncat
.
If you really, truly need a deep copy of the slice, you can call strndup
.
Is Checking for that One Byte Enough?
The program is certainly simpler if you can just search for that one byte. Unfortunately, in the twenty-first century, supporting ASCII doesn’t cut it, but the standard library’s support for internationalization is stuck in the ’90s (and Microsoft’s does not even support that). To work with UTF-8, you would in practice need to use some third-party library, such as libunicode
.
Fortunately, if searching for the space character (or other ASCII whitespace) is enough, the same code will also run correctly on UTF-8 input. I’ll assume that’s the case for now.
Putting it All Together
First, something simpler and closer to your code than my original solutions.
With my suggested API, the signature of trim_string
becomes:
str_cslice_t string_trim( const char* const s, const size_t n )
If we add a function to convert a null-terminated string to_str_cslice
, we decompose the problem into converting, and then calling a function to trim both ends of a string slice. The first is an important utility function to have anyway, and the second is simpler to write, since it doesn’t need to allocate any memory, copy anything or check the length.
#include <assert.h>
#include <stdbool.h>
#include <stdlib.h>
#include <string.h>
/* A const string slice: Note that n is the actual length of the skice.
* Slices ARE NOT null-terminated.
*/
typedef struct str_cslice_t {
const char* s;
size_t n;
} str_cslice_t;
const str_cslice_t EMPTY_STR_CSLICE = { NULL, 0 };
/* Trims leading and trailing whitespace from a str_cslice_t.
*/
str_cslice_t str_cslice_trim( const str_cslice_t input )
{
if (input.s && input.n) {
const char* const end = input.s + input.n;
const char* first = input.s;
while ( first < end && *first == ' ' ) {
++first;
}
// We previously checked that n > 0.
const char *last = end - 1;
while ( last > first && *last == ' ' ) {
--last;
}
if (last > first || first > input.s) {
const str_cslice_t result = { first,
(size_t)(last - first + 1) };
return result;
}
}
return EMPTY_STR_CSLICE;
}
/* Converts a null-terminated string with a maximum length of n to a
* str_cslice_t.
*/
str_cslice_t to_str_cslice( const char* const s, const size_t n )
{
const size_t actual = strlen(s);
const str_cslice_t result = { s, actual < n ? actual : n };
return result;
}
/* Returns either a slice of the input string that does not include any
* leading and trailing spaces, or an empty slice if that substring is empty.
* The string is null-terminated, but its maximum length is n.
*/
str_cslice_t string_trim( const char* const s, const size_t n )
{
return str_cslice_trim(to_str_cslice(s, n));
}
#include <limits.h>
#include <stdio.h>
// Not re-entrant:
int main(void)
{
{
static const char input[] = " foo bar ";
const str_cslice_t trim_both = string_trim( input, sizeof(input) );
assert( trim_both.n <= INT_MAX );
printf( "\"%.*s\"\n", (int)trim_both.n, trim_both.s );
}
{
static const char input[] = "foo bar ";
const str_cslice_t trim_right = string_trim( input, sizeof(input) );
assert( trim_right.n <= INT_MAX );
printf( "\"%.*s\"\n", (int)trim_right.n, trim_right.s );
}
{
static const char input[] = " foo bar";
const str_cslice_t trim_left = string_trim( input, UINT_MAX );
assert( trim_left.n <= INT_MAX );
printf( "\"%.*s\"\n", (int)trim_left.n, trim_left.s );
}
{
static const char input[] = " ";
const str_cslice_t empty = string_trim( input, sizeof(input) );
assert(empty.n == 0);
printf( "\"%.*s\"\n", (int)empty.n, empty.s );
}
{
static const char input[] = " ! ";
const str_cslice_t singleton = string_trim( input, sizeof(input) );
assert(singleton.n == 1);
printf( "\"%.*s\"\n", (int)singleton.n, singleton.s );
}
{
const str_cslice_t should_fail = string_trim( "", 0 );
assert( !should_fail.s && !should_fail.n );
}
{
const str_cslice_t should_fail = string_trim( NULL, UINT_MAX );
assert( !should_fail.s && !should_fail.n );
}
return EXIT_SUCCESS;
}
I include a simple test harness at the bottom, but it does not have complete code coverage. Caveat emptor.
A Refactored Version
I belatedly realized that I could clean up the code immensely by factoring out the trim-left and trim-right functions. We can also make both of these library functions take and return our str_cslice_t
structures, same as before. The string_trim
function is now the composition of three smaller functions, all of which are somewhat useful on their own: converting a null-terminated string into a string slice, trimming it on the left, and trimming it on the right.
Because I’m weird, I like to code in a functional style even in C. This can get complicated (as in my previous edits), but here, with the functions small enough, it becomes very straightforward. One advantage of this approach is that the entire program can be written with static single assignments. The local state is always updated together, at the same time. This eliminates several large categories of bugs.
Since this approach makes heavy use of tail recursion, and C was not designed for that, you need to make sure that the compiler optimizes tail calls. Clang and ICX have an extension for this, so I #define MUSTTAIL
to expand to that attribute on those compilers, or to a no-op on other compilers. On GCC, you will need to compile this code with -O2
, -O3
, -Os
, or at least -foptimize-sibling-calls
.
#include <assert.h>
#include <stdbool.h>
#include <stdlib.h>
#include <string.h>
#if __clang__ || __INTEL_LLVM_COMPILER
# define MUSTTAIL __attribute((musttail))
#else
# define MUSTTAIL /**/
#endif
/* A const string slice: Note that n is the actual length of the skice.
* Slices ARE NOT null-terminated.
*/
typedef struct str_cslice_t {
const char* s;
size_t n;
} str_cslice_t;
const str_cslice_t EMPTY_STR_CSLICE = { NULL, 0 };
/* Trims leading whitespace from a slice.
*/
str_cslice_t str_cslice_trim_left( const str_cslice_t input )
{
if (!input.s || !input.n)
return EMPTY_STR_CSLICE;
if (*input.s != ' ')
return input;
MUSTTAIL return str_cslice_trim_left((struct str_cslice_t){ input.s+1, input.n-1 });
}
/* Trims trailing whitespace from a slice.
*/
str_cslice_t str_cslice_trim_right( const str_cslice_t input )
{
if (!input.s || !input.n)
return EMPTY_STR_CSLICE;
if (input.s[input.n-1] != ' ')
return input;
MUSTTAIL return str_cslice_trim_right((struct str_cslice_t){ input.s, input.n-1 });
}
/* Trims leading and trailing whitespace from a str_cslice_t.
*/
str_cslice_t str_cslice_trim( const str_cslice_t input )
{
MUSTTAIL return str_cslice_trim_right(str_cslice_trim_left(input));
}
/* Converts a null-terminated string with a maximum length of n to a
* str_cslice_t.
*/
str_cslice_t to_str_cslice( const char* const s, const size_t n )
{
const size_t actual = strlen(s);
const str_cslice_t result = { s, actual < n ? actual : n };
return result;
}
/* Returns either a slice of the input string that does not include any
* leading and trailing spaces, or an empty slice if that substring is empty.
* The string is null-terminated, but its maximum length is n.
*/
str_cslice_t string_trim( const char* const s, const size_t n )
{
return str_cslice_trim(to_str_cslice(s, n));
}
#include <limits.h>
#include <stdio.h>
// Not re-entrant:
int main(void)
{
{
static const char input[] = " foo bar ";
const str_cslice_t trim_both = string_trim( input, sizeof(input) );
assert( trim_both.n <= INT_MAX );
printf( "\"%.*s\"\n", (int)trim_both.n, trim_both.s );
}
{
static const char input[] = "foo bar ";
const str_cslice_t trim_right = string_trim( input, sizeof(input) );
assert( trim_right.n <= INT_MAX );
printf( "\"%.*s\"\n", (int)trim_right.n, trim_right.s );
}
{
static const char input[] = " foo bar";
const str_cslice_t trim_left = string_trim( input, UINT_MAX );
assert( trim_left.n <= INT_MAX );
printf( "\"%.*s\"\n", (int)trim_left.n, trim_left.s );
}
{
static const char input[] = " ";
const str_cslice_t empty = string_trim( input, sizeof(input) );
assert(empty.n == 0);
printf( "\"%.*s\"\n", (int)empty.n, empty.s );
}
{
static const char input[] = " ! ";
const str_cslice_t singleton = string_trim( input, sizeof(input) );
assert(singleton.n == 1);
printf( "\"%.*s\"\n", (int)singleton.n, singleton.s );
}
{
const str_cslice_t should_fail = string_trim( "", 0 );
assert( !should_fail.s && !should_fail.n );
}
{
const str_cslice_t should_fail = string_trim( NULL, UINT_MAX );
assert( !should_fail.s && !should_fail.n );
}
return EXIT_SUCCESS;
}
ICX 2022 with -std=c17 -O3 -march=x86-64-v3
is able to inline these calls, resulting in extremely efficient code. Each of the test cases compiles to a block like the following:
mov edi, offset .L.str.2
mov edx, offset main.input+3
mov esi, 7
xor eax, eax
call printf
The non-inlined version optimizes into a tight loop as well, and this version does not use the heap at all, nor make a deep copy of any string.
One last footnote: if you are extremely security-conscious, you might want to use strnlen_s
rather than strlen
in the to_str_cslice
function. This version is very fast and portable, but it’s theoretically possible that an unterminated string could make strlen
run off the end of the buffer onto some unreadable page of memory and crash the program.
strdup
and/or therealloc
is unnecessary. \$\endgroup\$string_trim
does not return anything? \$\endgroup\$realloc()
andmalloc()
. \$\endgroup\$