[MPlayer-cvslog] r30900 - trunk/loader/win32.c

[Diego Biurrun]

On Mon, Mar 15, 2010 at 04:28:48PM +0100, Steinar H. Gunderson wrote:
> On Mon, Mar 15, 2010 at 04:24:49PM +0100, Diego Biurrun wrote:
> >> +#if 1
> >> +    // leave it here for testing win9x-only codecs
> >> +    c->dwPlatformId=VER_PLATFORM_WIN32_WINDOWS;
> >> +    strcpy(CSDVersion, " B");
> >> +#else
> >> +    c->dwPlatformId=VER_PLATFORM_WIN32_NT; // let's not make DLL assume that it can read CR* registers
> >> +    strcpy(CSDVersion, "Service Pack 3");
> >> +#endif
> > Ugh, do we need to keep such ugly debug code?
> 
> It was copied from the *A version. Should I remove the #else arm from both
> functions?

You decide, but I think the loader code could use some cleanup, so I'm
in favor.

> >> +    MultiByteToWideChar(65001, 0x0, CSDVersion, -1, c->szCSDVersion, 128);
> >> +    dbgprintf("  Major version: %d\n  Minor version: %d\n  Build number: 0x%08x\n"
> >> +             "  Platform Id: %s\n Version string: '%s'\n",
> >> +      c->dwMajorVersion, c->dwMinorVersion, c->dwBuildNumber,
> >> +             (c->dwPlatformId==VER_PLATFORM_WIN32_WINDOWS ? "VER_PLATFORM_WIN32_WINDOWS" :
> >> +       (c->dwPlatformId==VER_PLATFORM_WIN32_NT ? "VER_PLATFORM_WIN32_NT" : "Unknown")),
> >> +             CSDVersion);
> > The indentation of this function call is .. interesting.
> 
> It's actually not that inconsistent,

Everything is relative, but I would suggest something like

  dbgprintf("  Major version: %d\n  Minor version: %d\n  Build number: 0x%08x\n"
            "  Platform Id: %s\n Version string: '%s'\n",
            c->dwMajorVersion, c->dwMinorVersion, c->dwBuildNumber,
            (c->dwPlatformId == VER_PLATFORM_WIN32_WINDOWS ? "VER_PLATFORM_WIN32_WINDOWS" :
                (c->dwPlatformId == VER_PLATFORM_WIN32_NT ? "VER_PLATFORM_WIN32_NT" : "Unknown")),
            CSDVersion);

> but having multiple nested ?: inside a
> printf might not have been the best of ideas...

Yes.

Diego