From 03cedbb4ccaaa7a1f562156a2a7724ac02429803 Mon Sep 17 00:00:00 2001 From: Kp Date: Sat, 27 May 2023 13:19:31 +0000 Subject: [PATCH] Remove default-zero for `physfsrwops_seek` variable pos This default-zero allowed the bug introduced in 68b47307b4f24c3905c740f79f759e97d5b6ff3c (and fixed in 4c371734b53bf1784d226a0f09c94fcbdbb00969) not to generate a compiler warning for `-Wuninitialized`, since it was initialized to `0`. However, `0` is not a correct starting value for this variable. Remove the initialization and require every code path to assign a meaningful value. Switch to use `std::in_range` to validate that no truncation occurs, and trust that the compiler will optimize this out when the type passed to `std::in_range` can represent all values that the argument can represent. --- common/misc/physfsrwops.cpp | 44 +++++++++++++++++++++++-------------- 1 file changed, 27 insertions(+), 17 deletions(-) diff --git a/common/misc/physfsrwops.cpp b/common/misc/physfsrwops.cpp index 46041956f..6c0dd5f58 100644 --- a/common/misc/physfsrwops.cpp +++ b/common/misc/physfsrwops.cpp @@ -45,7 +45,7 @@ namespace { static SDL_RWops_callback_seek_position physfsrwops_seek(SDL_RWops *rw, const SDL_RWops_callback_seek_position offset, const int whence) { PHYSFS_File *handle = reinterpret_cast(rw->hidden.unknown.data1); - SDL_RWops_callback_seek_position pos = 0; + SDL_RWops_callback_seek_position pos; if (whence == SEEK_SET) { @@ -62,21 +62,35 @@ static SDL_RWops_callback_seek_position physfsrwops_seek(SDL_RWops *rw, const SD return(-1); } /* if */ -#if SDL_MAJOR_VERSION == 1 - pos = static_cast(current); - if (static_cast(pos) != current) + if (!std::in_range(current)) { SDL_SetError("Can't fit current file position in an int!"); return(-1); } /* if */ -#else - pos = current; -#endif - if (offset == 0) /* this is a "tell" call. We're done. */ - return(pos); + return current; - pos += offset; + /* When `SDL_RWops_callback_seek_position` is `int`, this assignment + * converts to a narrower type, but the call to std::in_range above + * rejected any values for which the narrowing would change the + * observed value. An assignment which prohibits narrowing would be + * ill-formed, since the compile-time check for narrowing is + * context-free and assumes the worst case. Therefore, an + * initialization that prohibited narrowing would trigger a compile + * error. + * + * When `SDL_RWops_callback_seek_position` is `Sint64`, then on + * x86_64-w64-mingw32, + * `static_cast(v)` triggers + * `-Wuseless-cast`. + * + * When `SDL_RWops_callback_seek_position` is `Sint64`, then on + * x86_64-pc-linux-gnu, + * `static_cast(v)` is accepted + * without issue. + */ + const SDL_RWops_callback_seek_position rwcurrent = current; + pos = rwcurrent + offset; } /* else if */ else if (whence == SEEK_END) @@ -88,18 +102,14 @@ static SDL_RWops_callback_seek_position physfsrwops_seek(SDL_RWops *rw, const SD return(-1); } /* if */ -#if SDL_MAJOR_VERSION == 1 - pos = static_cast(len); - if (static_cast(pos) != len) + if (!std::in_range(len)) { SDL_SetError("Can't fit end-of-file position in an int!"); return(-1); } /* if */ -#else - pos = len; -#endif - pos += offset; + const SDL_RWops_callback_seek_position rwlen = len; + pos = rwlen + offset; } /* else if */ else