JoeNotCharles reports that gcc-12 (architecture unspecified) warns
because `newmenu::process_until_closed` stores the address of a stack
local into a heap-allocated structure. Switch to a less efficient
implementation that does not provoke a compiler warning:
- Store `shared_ptr<int>`, not `int *`, in the `newmenu`
- `shared_ptr::operator*()` and `shared_ptr::operator bool()` allow most
use sites to be unchanged relative to a bare pointer
- Load the return value from the `shared_ptr<int>`. If the newmenu
terminated before return, as should always happen, this is a slightly
less efficient version of the same code as before. If the newmenu was
still open despite its `exists` flag claiming otherwise, then the
returned value may be incorrect, but the read will be well-formed, the
`newmenu`'s eventual write will write to the heap-backed int (rather
than writing into the stack allocated to
`newmenu::process_until_closed`), and the memory will be freed when
both `process_until_closed` has returned and the `newmenu` has been
destroyed.
Reported-by: JoeNotCharles <10a2b2d337>
gcc-12 does enough escape analysis to notice that
newmenu::process_until_closed stores the address of a stack local into a
heap-allocated structure, but not enough analysis to notice that the
stack variable always outlives the escaped address. The compiler then
warns about the address escaping the local scope. Reworking the calling
code not to do this is somewhat invasive, and gcc seems unlikely to
change behavior. Switch to a less efficient implementation that does
not provoke a compiler warning:
- Store a shared_ptr<bool> in the object
- In the object's destructor, write through the pointer to clear the
shared boolean
- In the caller, store a copy of the shared_ptr<bool> as a local, and
use that copy to monitor the shared boolean
This is similar to a change proposed by JoeNotCharles
<10a2b2d337>,
but differs in its details. Among other things, this version takes the
opportunity to move the variable out to a mixin, so that only windows
which expect to be tracked can be tracked. Previously, all windows were
capable of this, even though most never needed it.
JoeNotCharles reported
<1f1903a3b9>
an overflow in `net_udp_send_mdata_direct`. This overflow is impossible
as currently written, because it can occur only if
`multi_send_data_direct` passes an oversized buffer to
`net_udp_send_mdata_direct`, and no messages are large enough to trigger
this. JoeNotCharles proposed adding a runtime check to abort the
program if this happens. Instead, this commit adds a compile-time check
to detect use of an excessively large input buffer.
JoeNotCharles reports that gcc-12 (architecture unspecified) warns about
a possible truncation when constructing a path. This would only occur
if the user had a path that was almost PATH_MAX deep (4096 bytes on most
platforms). Add safety checks around this, and around an underflow if
this path were somehow reached while `newpath` was shorter than `sep`.
This is loosely based on a proposed commit by JoeNotCharles, but
rewritten to update comments and adjust the code flow.
Reported-by: JoeNotCharles <edfb3c4b77>
Construct the nm_messagebox_tie directly, without use of a macro. This
produces simpler compiler error messages when nm_messagebox is called
incorrectly.
Per SDL documentation, SDL will generate an SDL_QUIT event when
Command-Q is used. Rebirth already handles SDL_QUIT, so there is no
need to explicitly detect Command-Q.
The new resampler sets upFactor to either `2` (for 22Khz sounds) or
`4` (for 11Khz sounds). However, the logic to pick which coefficient
table to use tested for `0` versus non-zero, so it always picked the
coefficient table meant for 22Khz sounds, even when the sample was an
11Khz sound. This caused a high pitched ringing in sampled sounds.
Switch to use an `enum class` to prevent using zero-vs-nonzero tests
without a cast. Change the existing test to test for the 22Khz `enum`
value, as originally intended. In testing, this eliminated the ringing
effect in 11Khz sounds.
Identified-by: Chris <https://forum.zdoom.org/viewtopic.php?p=1234989> (in response to a request by KynikossDragonn)
Fixes: 5165efbc46 ("Use custom fixed-point audio resampler: - Hopefully fixes issues with poor quality resampling when using SDL_AudioCVT - Converts 11025 Hz or 22050 Hz samples to the default 44100 Hz outputs - Uses high order brick wall filter like Sound Blaster 16")
In any given run of the program, either the SDL_mixer code will be used,
or it will not be used. `digi_sample_rate` only needs to vary if a
single run both uses SDL_mixer and avoids it. Make `digi_sample_rate` a
`static const` with an appropriate value for each path.
Add the SDL_mixer-based resampler as a compile-time alternative, which
can be chosen by setting `-DDXX_FEATURE_INTERNAL_RESAMPLER=0`. Keep the
new internal resampler as the default.
This allows the compiler to see and perform basic checks on the call
even in builds with `DXX_WORDS_NEED_ALIGNMENT == 0`.
Also, fix a missing address-of operator in one invocation of
`align_polygon_model_data`.
Reported-by: Brunnis <https://github.com/dxx-rebirth/dxx-rebirth/issues/687> (missing address-of operator)
Fixes: 43e1c841f0 ("Pass polymodel& to polymodel_read")
Switch from using a macro to capture __FILE__,__LINE__ to using
__builtin_FILE(),__builtin_LINE(). Make the event an explicit argument,
instead of assuming it is a variable named `event`. Move the
implementation out of line.
