Re: [PATCH] ddraw: Fix use of uninitialized memory.

[Stefan Dösinger]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Am 2016-01-14 um 20:49 schrieb Thomas Faller:
> +    if ((desc->dwFlags & DDSD_LPSURFACE)
> +            && ((desc->ddsCaps.dwCaps & DDSCAPS_ALLOCONLOAD) || !desc->lpSurface))
>          desc->dwFlags &= ~DDSD_LPSURFACE;
> I'd say report this to valgrind as well. If DDSD_LPSURFACE isn't set in
> the first place then randomly unsetting it doesn't really hurt.
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v2

iQIcBAEBCAAGBQJWmBWcAAoJEN0/YqbEcdMwuuoP/R/psF/dtrnIb0tQH/JP9Ikg
kbDNd1NUuwx8QEOKNevopq/ii24whN0VpgJRSKMslY3PvSH6WnWDq9VT3PxSECsc
dhfESXJBPSHDfuyzRhVrtwNQvwKrotpUXQ1bIW3AKKsJRr8yOxS3QkYMbt9zMItO
YMqy1MP1315Kp6o51LW3iMANipqpmSUfTIjPWrHj7nMKgNTcTbrqOpGHKezxw1u7
ox10hCEwVj5429drUej/Q/nQ1OoAluWAdH0hWZhAF4zi5O+1wwH7I+KwqbL6a/zK
7djvDv+V3EeHxuax3ljwtMoa6Vxwna1vpGSCTHN8N/gNZcFR5v2ne+i+rkGoFQ3V
8u5Lhb1CMaAsTrzBnw+jX2JKSsbpgzFO8nVlDEr2G/cSvf/wq1DaNRpjmFMCExYP
0fs1PzHf4jBubqVChycSbCT1AGEq29s2msn6ucomoTdM1kctRFKjoxPk76iJKE86
mSWBkcmjRSaDBuHt76xjpL9NPEO3CO6lJbtLslEo33kCbsdrv8i+AVGBBvWMf9Oj
/bVieoGqk5djdi0bggrhspKGCsVZURYrfpgFv/FIq66Vs7SzEgZoXABoPkjugwtB
DLPF2s1WjUqAq0CYWzgLOprvbFI2DPz/VtbaBoJ0GYr3bZJca+ZV8njXsQUMi52b
PtFvXa2A4KJOm18CrCm4
=gjon
-----END PGP SIGNATURE-----

[PATCH] ddraw: Fix use of uninitialized memory.

[Henri Verbeet]

On 14 January 2016 at 22:39, Stefan Dösinger <stefandoesinger(a)gmail.com> wrote:
> Am 2016-01-14 um 20:49 schrieb Thomas Faller:
> > +    if ((desc->dwFlags & DDSD_LPSURFACE)
> > +            && ((desc->ddsCaps.dwCaps & DDSCAPS_ALLOCONLOAD) || !desc->lpSurface))
> >          desc->dwFlags &= ~DDSD_LPSURFACE;
> > I'd say report this to valgrind as well. If DDSD_LPSURFACE isn't set in
> > the first place then randomly unsetting it doesn't really hurt.
> > Yeah, this is really more about making Valgrind shut-up than fixing
> > anything. I don't think Valgrind has enough information to be able to
> > tell it's actually ok though.

The bigger issue is that it looks like the code is just wrong though.
It's one of those things that has zero tests (because, ddraw), so I
tried adding some, and DDSD_LPSURFACE with a NULL lpSurface pretty
much returns DDERR_INVALIDPARAMS regardless of DDSCAPS_ALLOCONLOAD or
not.

[PATCH] ddraw: Fix use of uninitialized memory.

[Thomas Faller]

Am 15.01.2016 um 16:52 schrieb Henri Verbeet:
> On 14 January 2016 at 22:39, Stefan Dösinger <stefandoesinger(a)gmail.com> wrote:
> > Am 2016-01-14 um 20:49 schrieb Thomas Faller:
> > > +    if ((desc->dwFlags & DDSD_LPSURFACE)
> > > +            && ((desc->ddsCaps.dwCaps & DDSCAPS_ALLOCONLOAD) || !desc->lpSurface))
> > >           desc->dwFlags &= ~DDSD_LPSURFACE;
> > > I'd say report this to valgrind as well. If DDSD_LPSURFACE isn't set in
> > > the first place then randomly unsetting it doesn't really hurt.
> > > Yeah, this is really more about making Valgrind shut-up than fixing
> > > anything. I don't think Valgrind has enough information to be able to
> > > tell it's actually ok though.
>
> The bigger issue is that it looks like the code is just wrong though.
> It's one of those things that has zero tests (because, ddraw), so I
> tried adding some, and DDSD_LPSURFACE with a NULL lpSurface pretty
> much returns DDERR_INVALIDPARAMS regardless of DDSCAPS_ALLOCONLOAD or
> not.

Yes, valgrind doesn't understand the underlying logic.
lpSurface is only valid if DDSD_LPSURFACE is set, but valgrind can't 
know this.