Re: [PATCH] d3dx9_36: Set compilation_errors to NULL when no error encountered + tests.

[Rico Schüller]

On 11.06.2013 22:08, Christian Costa wrote:
> Fixes bug 26598.
>
> dlls/d3dx9_36/effect.c       |    4 ++++
>   dlls/d3dx9_36/tests/effect.c |   17 +++++++++++++++++
>   2 files changed, 21 insertions(+)
>
> diff --git a/dlls/d3dx9_36/effect.c b/dlls/d3dx9_36/effect.c
> index 1924c07..bab4560 100644
> --- a/dlls/d3dx9_36/effect.c
> +++ b/dlls/d3dx9_36/effect.c
> @@ -5792,6 +5792,10 @@ HRESULT WINAPI D3DXCreateEffectEx(struct IDirect3DDevice9 *device, const void *s
>   *effect = &object->ID3DXEffect_iface;
>
> - /* Must be set to NULL if no compilation error */
> - if (compilation_errors)
> -
>    *compilation_errors = NULL;
> -

No, this is wrong! Your test case doesn't cover all cases.

Cheers
Rico

[PATCH] d3dx9_36: Set compilation_errors to NULL when no error encountered + tests.

[Christian Costa]

2013/6/11 Rico Schüller
> On 11.06.2013 22:08, Christian Costa wrote:
> > Fixes bug 26598.
> >
> > dlls/d3dx9_36/effect.c       |    4 ++++
> >   dlls/d3dx9_36/tests/effect.c |   17 +++++++++++++++++
> >   2 files changed, 21 insertions(+)
> >
> > diff --git a/dlls/d3dx9_36/effect.c b/dlls/d3dx9_36/effect.c
> > index 1924c07..bab4560 100644
> > --- a/dlls/d3dx9_36/effect.c
> > +++ b/dlls/d3dx9_36/effect.c
> > @@ -5792,6 +5792,10 @@ HRESULT WINAPI D3DXCreateEffectEx(struct
> > IDirect3DDevice9 *device, const void *s
> >   *effect = &object->ID3DXEffect_iface;
> >
> > - /* Must be set to NULL if no compilation error */
> > - if (compilation_errors)
> > -
> >    *compilation_errors = NULL;
> > -
>
> No, this is wrong! Your test case doesn't cover all cases.
>
> Which cases? Ssource effects or possible parsing error of binary ones?

Anyway my intention was to fix only this particular case as we don't
support source effect yet.
I'll use a different approach.

[PATCH] d3dx9_36: Set compilation_errors to NULL when no error encountered + tests.

[Rico Schüller]

On 12.06.2013 10:31, Christian Costa wrote:
> 2013/6/11 Rico Schüller
> > On 11.06.2013 22:08, Christian Costa wrote:
> > > Fixes bug 26598.
> > >
> > > dlls/d3dx9_36/effect.c       |    4 ++++
> > >    dlls/d3dx9_36/tests/effect.c |   17 +++++++++++++++++
> > >    2 files changed, 21 insertions(+)
> > >
> > > diff --git a/dlls/d3dx9_36/effect.c b/dlls/d3dx9_36/effect.c
> > > index 1924c07..bab4560 100644
> > > --- a/dlls/d3dx9_36/effect.c
> > > +++ b/dlls/d3dx9_36/effect.c
> > > @@ -5792,6 +5792,10 @@ HRESULT WINAPI D3DXCreateEffectEx(struct
> > > IDirect3DDevice9 *device, const void *s
> > >    *effect = &object->ID3DXEffect_iface;
> > >
> > > - /* Must be set to NULL if no compilation error */
> > > - if (compilation_errors)
> > > -
> > >    *compilation_errors = NULL;
> > > -
> >
> > No, this is wrong! Your test case doesn't cover all cases.
> >
> > Which cases? Ssource effects or possible parsing error of binary ones?
>
> Anyway my intention was to fix only this particular case as we don't
> support source effect yet.
> I'll use a different approach.

The position of setting the compilation_errors is wrong. It should be 
before all other sanity checks. The intention is fine. See the attached 
patch...

Yes this should only fix this particular case. I don't think the 
compiler is ready to get called, yet.

Cheers
Rico