Skip to content

Commit c268f73

Browse files
committed
libretro-common/string: drop redundant NUL write in string_tokenize
Reported by gcc 15.2.0 (msys2 / MinGW) building libretro-net-retropad under -Wstringop-overflow: stdstring.c: In function 'string_tokenize': stdstring.c:500:16: warning: writing 1 byte into a region of size 0 [-Wstringop-overflow=] 500 | token[_len] = '\0'; | ~~~~~~~~~~~~^~~~~~ The flagged write is redundant. string_tokenize allocates malloc((_len + 1) * sizeof(char)) and then calls strlcpy(token, str_ptr, _len + 1). Looking at the strlcpy contract in libretro-common/compat/compat_strl.c:31-41: size_t __len = _len < len - 1 ? _len : len - 1; memcpy(s, in, __len); s[__len] = '\0'; strlcpy already NUL-terminates within the `_len + 1` byte limit. Because _len was computed at lines 489-492 as either (delim_ptr - str_ptr) -- strictly <= strlen(str_ptr) -- or strlen(str_ptr) directly, _len <= strlen(str_ptr) is always true, so strlcpy's __len resolves to _len and the terminator lands at token[_len]. The token[_len] = '\0' line just rewrites the byte strlcpy already wrote. gcc 15's tightened -Wstringop-overflow analyser cannot constrain the relationship between _len and the malloc'd size (the size expression `(_len + 1) * sizeof(char)` involves a multiply that the analyser doesn't fold through to the bounds check on token[_len]), so it warns under the assumption that _len could exceed the malloc'd size by one. Removing the redundant write silences the warning without changing semantics. Not a strlcpy_append situation: that helper (78c52ab) is for chained appends of the form _len += strlcpy(s + _len, src, len - _len); This is a single copy into a freshly-malloc'd buffer -- the right fix is to delete the redundant trailing write, not to swap idioms. * libretro-common/string/stdstring.c Drop the redundant token[_len] = '\0' that follows the strlcpy in string_tokenize, with an inline comment explaining the strlcpy contract that makes the line dead and the -Wstringop-overflow interaction that made it actively harmful. Verified locally: - The five string_tokenize behaviours that callers rely on (basic comma split, empty token at start, multi-char delimiter, no-delimiter pass-through, empty input) all pass under -fsanitize=address with the redundant write removed. - Cannot reproduce the gcc 15 warning on local gcc 13.3 (warning is gcc-15-specific), but verified the local build stays clean under -Wstringop-overflow=4 and -Werror after the change.
1 parent 5d08de4 commit c268f73

1 file changed

Lines changed: 8 additions & 2 deletions

File tree

libretro-common/string/stdstring.c

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -495,9 +495,15 @@ char* string_tokenize(char **str, const char *delim)
495495
if (!(token = (char *)malloc((_len + 1) * sizeof(char))))
496496
return NULL;
497497

498-
/* Copy token */
498+
/* Copy token. strlcpy already NUL-terminates within the
499+
* `_len + 1` byte limit -- _len is bounded above by
500+
* strlen(str_ptr) (computed at lines 489-492), so the
501+
* terminator lands exactly at token[_len] without needing
502+
* a separate write here. The redundant token[_len] = '\0'
503+
* also tripped -Wstringop-overflow under some gcc
504+
* configurations (the analyser couldn't constrain _len
505+
* relative to the malloc'd size). */
499506
strlcpy(token, str_ptr, (_len + 1) * sizeof(char));
500-
token[_len] = '\0';
501507
/* Update input string pointer */
502508
*str = delim_ptr ? delim_ptr + delim_len : NULL;
503509
return token;

0 commit comments

Comments
 (0)