Re: winmm: Clear adwInfo when the struct MMIOINFO is initialized by MMIO_Open

[Andrew Eikum]

This looks OK to me. What do you think about zeroing the whole
structure instead of just some of the fields, just to prevent this
problem happening again in the future?

In either case,

Signed-off-by: Andrew Eikum <aeikum(a)codeweavers.com>

On Fri, Oct 23, 2015 at 06:24:02PM +0800, Bruno Jesus wrote:
> Signed-off-by: Bruno Jesus <00cpxxx(a)gmail.com>
>
> The program from bug 35872 registers an IO handler that test adwInfo
> and crashes because when MMIO_Open is called these values have
> garbage.
>
> Fixes https://bugs.winehq.org/show_bug.cgi?id=35872
> diff --git a/dlls/winmm/mmio.c b/dlls/winmm/mmio.c
> index ad73564..03cadd5 100644
> --- a/dlls/winmm/mmio.c
> +++ b/dlls/winmm/mmio.c
> @@ -596,6 +596,7 @@ static HMMIO MMIO_Open(LPSTR szFileName, MMIOINFO* refmminfo, DWORD dwOpenFlags,
>   mmioinfo.pIOProc = NULL;
>   mmioinfo.pchBuffer = NULL;
>   mmioinfo.cchBuffer = 0;
> +	memset(mmioinfo.adwInfo, 0, sizeof(mmioinfo.adwInfo));
>          is_unicode = FALSE;
>      }
>
> diff --git a/dlls/winmm/tests/mmio.c b/dlls/winmm/tests/mmio.c
> index fd5433c..48367ee 100644
> --- a/dlls/winmm/tests/mmio.c
> +++ b/dlls/winmm/tests/mmio.c
> @@ -599,12 +599,15 @@ static void test_mmioSetBuffer(char *fname)
>  static LRESULT CALLBACK mmio_test_IOProc(LPSTR lpMMIOInfo, UINT uMessage, LPARAM lParam1, LPARAM lParam2)
>  {
>      LPMMIOINFO lpInfo = (LPMMIOINFO) lpMMIOInfo;
> +    int i;
>
> switch (uMessage)
>      {
>      case MMIOM_OPEN:
>          if (lpInfo->fccIOProc == FOURCC_DOS)
>              lpInfo->fccIOProc = mmioFOURCC('F', 'A', 'I', 'L');
> +        for (i = 0; i < sizeof(lpInfo->adwInfo) / sizeof(*lpInfo->adwInfo); i++)
> +            ok(lpInfo->adwInfo[i] == 0, "[%d] Expected 0, got %u\n", i, lpInfo->adwInfo[i]);
>          return MMSYSERR_NOERROR;
>      case MMIOM_CLOSE:
>          return MMSYSERR_NOERROR;
> @@ -642,6 +645,18 @@ static void test_mmioOpen_fourcc(void)
>         mmio.lDiskOffset);
>      mmioClose(hmmio, 0);
>
> +    /* Same test with NULL info */
> +    memset(&mmio, 0, sizeof(mmio));
> +    hmmio = mmioOpenA(fname, NULL, MMIO_READ);
> +    mmioGetInfo(hmmio, &mmio, 0);
> +    ok(hmmio && mmio.fccIOProc == FOURCC_XYZ, "mmioOpenA error %u, got %4.4s\n",
> +            mmio.wErrorRet, (LPCSTR)&mmio.fccIOProc);
> +    ok(mmio.adwInfo[1] == 0, "mmioOpenA sent MMIOM_SEEK, got %d\n",
> +       mmio.adwInfo[1]);
> +    ok(mmio.lDiskOffset == 0, "mmioOpenA updated lDiskOffset, got %d\n",
> +       mmio.lDiskOffset);
> +    mmioClose(hmmio, 0);
> +
>      mmioInstallIOProcA(FOURCC_XYZ, NULL, MMIO_REMOVEPROC);
>
> memset(&mmio, 0, sizeof(mmio));
>

winmm: Clear adwInfo when the struct MMIOINFO is initialized by MMIO_Open

[Bruno Jesus]

On Friday, October 23, 2015, Andrew Eikum <aeikum(a)codeweavers.com
<javascript:_e(%7B%7D,'cvml','aeikum(a)codeweavers.com');>> wrote:
> This looks OK to me. What do you think about zeroing the whole
> structure instead of just some of the fields, just to prevent this
> problem happening again in the future?

I believe it makes sense, better to have 0 in all fields than garbage.

https://source.winehq.org/patches/data/115723

Thanks,
Bruno