[PATCH] dlls/{mciwave, winmm}: properly handle mmioRead/mmioWrite in case of errors

[Andrew Eikum]

On Mon, Mar 28, 2022 at 02:38:01PM +0200, Eric Pouech wrote:
> In C, an inequality comparison between a signed and an unsigned integer
> ends up with an unsigned comparison.
> So, force signed comparison to catch both case of errors and partial
> read operation.
> 
> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52628
> Signed-off-by: Eric Pouech <eric.pouech at gmail.com>
> 
> ---
>  dlls/mciwave/mciwave.c |    3 ++-
>  dlls/winmm/mmio.c      |    8 +++-----
>  dlls/winmm/playsound.c |    3 ++-

Let's do one patch per module.

>  3 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/dlls/mciwave/mciwave.c b/dlls/mciwave/mciwave.c
> index 1d9abce84a3..3bb271f66ec 100644
> --- a/dlls/mciwave/mciwave.c
> +++ b/dlls/mciwave/mciwave.c
> @@ -291,7 +291,8 @@ static	DWORD WAVE_mciReadFmt(WINE_MCIWAVE* wmw, const MMCKINFO* pckMainRIFF)
>      if (!pwfx) return MCIERR_OUT_OF_MEMORY;
>  
>      r = mmioRead(wmw->hFile, (HPSTR)pwfx, mmckInfo.cksize);
> -    if (r < sizeof(PCMWAVEFORMAT)) {
> +    /* forcing signed comparison to catch errors (negative value) as well */
> +    if (r < (LONG)sizeof(PCMWAVEFORMAT)) {

I don't like the extra comment, but I don't have a much better idea
either.

What do you think about something like this instead?

    if (r < 0 || r < sizeof(PCMWAVEFORMAT)) {
        HeapFree(GetProcessHeap(), 0, pwfx);
        return MCIERR_INVALID_FILE;
    }

It's still not great, but I think this makes it more clear how we are
using the API.

Andrew