Re: [PATCH] winmm: Load MCI drivers from MCI32 key.

[Andrew Eikum]

This looks good, but could you add some tests showing something that
was wrong before but is now fixed?

Andrew

On Tue, Jul 25, 2017 at 09:18:15PM -0500, Zebediah Figura wrote:
> MCI key/section is for 16-bit drivers, which can't be loaded by
> 32-bit winmm.
>
> Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com>
> ---
>  dlls/winmm/mci.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/dlls/winmm/mci.c b/dlls/winmm/mci.c
> index 56a537d..d770f6d 100644
> --- a/dlls/winmm/mci.c
> +++ b/dlls/winmm/mci.c
> @@ -66,10 +66,10 @@ WINE_DEFAULT_DEBUG_CHANNEL(mci);
>  #define MCI_MAGIC 0x0001
>
> /* MCI settings */
> -static const WCHAR wszHklmMci  [] = {'S','o','f','t','w','a','r','e','\\','M','i','c','r','o','s','o','f','t','\\','W','i','n','d','o','w','s',' ','N','T','\\','C','u','r','r','e','n','t','V','e','r','s','i','o','n','\\','M','C','I',0};
> +static const WCHAR wszHklmMci  [] = {'S','o','f','t','w','a','r','e','\\','M','i','c','r','o','s','o','f','t','\\','W','i','n','d','o','w','s',' ','N','T','\\','C','u','r','r','e','n','t','V','e','r','s','i','o','n','\\','M','C','I','3','2',0};
>  static const WCHAR wszNull     [] = {0};
>  static const WCHAR wszAll      [] = {'A','L','L',0};
> -static const WCHAR wszMci      [] = {'M','C','I',0};
> +static const WCHAR wszMci      [] = {'M','C','I','3','2',0};
>  static const WCHAR wszOpen     [] = {'o','p','e','n',0};
>  static const WCHAR wszSystemIni[] = {'s','y','s','t','e','m','.','i','n','i',0};
>
> -- 
> 2.7.4

[PATCH] winmm: Load MCI drivers from MCI32 key.

[Zebediah Figura]

On 07/26/2017 08:03 AM, Andrew Eikum wrote:
> This looks good, but could you add some tests showing something that
> was wrong before but is now fixed?
>
> Andrew

The application overwrites the value of MCI/avivideo in system.ini (to 
16-bit mciavi.drv). Is this appropriate for a test?

[PATCH] winmm: Load MCI drivers from MCI32 key.

[Andrew Eikum]

On Wed, Jul 26, 2017 at 06:45:32PM -0500, Zebediah Figura wrote:
> On 07/26/2017 08:03 AM, Andrew Eikum wrote:
> > This looks good, but could you add some tests showing something that
> > was wrong before but is now fixed?
>
> The application overwrites the value of MCI/avivideo in system.ini (to
> 16-bit mciavi.drv). Is this appropriate for a test?

Maybe I misunderstood the impact of this patch. Won't it affect things
like MCI_SYSINFO_QUANTITY, and other places that use those registry
keys?

Andrew

[PATCH] winmm: Load MCI drivers from MCI32 key.

[Zebediah Figura]

On 07/27/2017 08:40 AM, Andrew Eikum wrote:
> On Wed, Jul 26, 2017 at 06:45:32PM -0500, Zebediah Figura wrote:
> > On 07/26/2017 08:03 AM, Andrew Eikum wrote:
> > > This looks good, but could you add some tests showing something that
> > > was wrong before but is now fixed?
> >
> > The application overwrites the value of MCI/avivideo in system.ini (to
> > 16-bit mciavi.drv). Is this appropriate for a test?
>
> Maybe I misunderstood the impact of this patch. Won't it affect things
> like MCI_SYSINFO_QUANTITY, and other places that use those registry
> keys?
>
> Andrew

Yes, and that is the correct behaviour. I can write tests to demonstrate 
this if desired.