[PATCH v2] winmm: Don't crash in waveOutOpen when nSamplesPerSec is 0 and add tests

[Andrew Eikum]

Hi Fabian, some comments below.

On Sun, Jul 29, 2018 at 10:58:02PM +0200, Fabian Maurer wrote:
> Fixes bug 45530
> 

We recently decided to use a standard format for bug numbers:

    https://wiki.winehq.org/Submitting_Patches#The_commit_message

e.g.:

    Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=45530

> diff --git a/dlls/winmm/tests/wave.c b/dlls/winmm/tests/wave.c
> index b402e21917..cfc4c726be 100644
> --- a/dlls/winmm/tests/wave.c
> +++ b/dlls/winmm/tests/wave.c
> @@ -1708,6 +1708,43 @@ static void test_PlaySound(void)
>      DeleteFileA(test_file);
>  }
>  
> +static void test_waveOutOpen_invalid_parameter(void)
> +{
> +    HWAVEOUT handle;
> +    MMRESULT ret;
> +    WAVEFORMATEX wfx;
> +    WAVEFORMATEX const const_wfx = {
> +        .wFormatTag = WAVE_FORMAT_PCM,
> +        .nChannels = 1,
> +        .nSamplesPerSec = 11025,
> +        .nBlockAlign = 1,
> +        .nAvgBytesPerSec = 11025 * 1,
> +        .wBitsPerSample = 8,
> +        .cbSize = 0
> +    };
> +

Unfortunately we care about ancient compilers, so we can't do struct
initialization like this.

> @@ -1715,4 +1752,5 @@ START_TEST(wave)
>      test_sndPlaySound();
>      test_fragmentsize();
>      test_PlaySound();
> +    test_waveOutOpen_invalid_parameter();

Instead of creating a new top-level test and using the 0th device, how
about putting this someplace in wave_out_test_device? It has a bunch
of format tests near the bottom, I think you could add the INVALPARAM
ones down there.

> diff --git a/dlls/winmm/waveform.c b/dlls/winmm/waveform.c
> index 045bf4ac20..58f830dad9 100644
> --- a/dlls/winmm/waveform.c
> +++ b/dlls/winmm/waveform.c
> @@ -1106,6 +1106,12 @@ static LRESULT WINMM_OpenDevice(WINMM_Device *device, WINMM_OpenInfo *info,
>              WARN("Fixing bad nAvgBytesPerSec (%u)\n", device->orig_fmt->nAvgBytesPerSec);
>              device->orig_fmt->nAvgBytesPerSec  = device->orig_fmt->nSamplesPerSec * device->orig_fmt->nBlockAlign;
>          }
> +
> +        if (info->format->nSamplesPerSec == 0)
> +        {
> +            ret = MMSYSERR_INVALPARAM;
> +            goto error;
> +        }

I think you could move this check up above the allocation and other
format checks. No need to do that work if the format is busted.

Thanks,
Andrew