Eliminated build warnings

[logiclrd]

This PR eliminates build warnings on a Linux build using GCC version 13.2.0-23ubuntu4, as of September 2024.

• In the code that uses FluidSynth, there are calls to functions for setting parameters that have been deprecated. This PR splits these into calls to the recommended replacements.
• There are several calls to fread that ignore the return value. This PR changes the code so that it still ignores the return value, but it does it explicitly with a if statement with an empty body.
• Marked a couple of switch case blocks as being explicit fallthrough to the next case.

One of the warnings was indicative of an actual bug, I believe. In Ym2612_Nuked.cpp, the definition of the Ym2612_Nuked_Emu::reset function calls OPN2_Reset on chip_r only if chip_r is NULL. I'm pretty sure this should be the other way around, and making it so that it only calls it when chip_r is not NULL eliminates a warning.

With these changes, which I believe do not alter functionality in any way, the ZMusic build runs through with no warnings at all. Could possibly add -Werror to the build.

[logiclrd]

Looks like on the Windows build, the use of [[fallthrough]] requires explicit opt-in to C++17. It has introduced a couple of new warnings into the Windows build. Non-breaking, though.

[logiclrd]

A bunch of warnings still present in the MacOS build. I'm not set up to sort those out. :-P

[coelckers]

I do not like where this is going, especially the if (fread...) constructs.

[logiclrd]

I do not like where this is going, especially the if (fread...) constructs.

That is literally the recommended way to indicate that you are explicitly not using the return value, from what I have read.

It used to be to cast the return value to void, but that apparently stopped working (in GCC) a while back. So, some people then started doing (void)(ignored_return_value() + 1). This is obviously ridiculous.

Can you explain what you mean more generally by not liking where things are going? I mean, "where it's going" is being able to treat warnings as errors. Isn't that generally a good thing?

[logiclrd]

Also, the code is presently calling a bunch of deprecated Fluid Synth methods. The message attached to the deprecation explicitly says to call the per-property methods instead

 * @return #FLUID_OK on success, #FLUID_FAILED otherwise
 * @deprecated Use the individual reverb setter functions in new code instead.
 */
int
fluid_synth_set_reverb(fluid_synth_t *synth, double roomsize, double damping,
                       double width, double level)
{

[logiclrd]

Most of the other changes are literally just indenting fixes, but this one is an actual bug fix:

 void Ym2612_Nuked_Emu::reset()
 {
    Ym2612_NukedImpl::ym3438_t *chip_r = reinterpret_cast<Ym2612_NukedImpl::ym3438_t*>(impl);
-   if ( !chip_r ) Ym2612_NukedImpl::OPN2_Reset( chip_r, static_cast<Bit32u>(prev_sample_rate), static_cast<Bit32u>(prev_clock_rate) );
+   if ( chip_r ) Ym2612_NukedImpl::OPN2_Reset( chip_r, static_cast<Bit32u>(prev_sample_rate), static_cast<Bit32u>(prev_clock_rate) );
 }

This code is currently literally saying, "Only call OPN2_Reset on the chip data structure if it's a NULL reference."

A static code analysis warning results, because the reset process involves dereferencing the pointer that's passed in. A warning found this actual bug.

[logiclrd]

I have pushed a commit that handles the ignored fread return values in a different way. I believe you will see it if you reopen the PR.