Re: [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 <kgbric...@web.de>
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
commit 04974b07438954ef28578b9527beff413ba9a16e
Author: Rico Schüller <kgbric...@web.de>
Date:   Wed Jun 12 11:54:46 2013 +0200

    d3dx9: Set compilation_errors to NULL in D3DXCreateEffectEx().

diff --git a/dlls/d3dx9_36/effect.c b/dlls/d3dx9_36/effect.c
index 464124d..b7d22fa 100644
--- a/dlls/d3dx9_36/effect.c
+++ b/dlls/d3dx9_36/effect.c
@@ -5789,6 +5789,9 @@ HRESULT WINAPI D3DXCreateEffectEx(struct IDirect3DDevice9 *device, const void *s
     FIXME("(%p, %p, %u, %p, %p, %p, %#x, %p, %p, %p): semi-stub\n", device, srcdata, srcdatalen, defines, include,
         skip_constants, flags, pool, effect, compilation_errors);
 
+    if (compilation_errors)
+        *compilation_errors = NULL;
+
     if (!device || !srcdata)
         return D3DERR_INVALIDCALL;
 
diff --git a/dlls/d3dx9_36/tests/effect.c b/dlls/d3dx9_36/tests/effect.c
index 7f238be..6626aa8 100644
--- a/dlls/d3dx9_36/tests/effect.c
+++ b/dlls/d3dx9_36/tests/effect.c
@@ -2662,6 +2662,26 @@ static void test_effect_variable_names(IDirect3DDevice9 *device)
     ok(!count, "Release failed %u\n", count);
 }
 
+static void test_effect_compilation_errors(IDirect3DDevice9 *device)
+{
+    ID3DXEffect *effect;
+    ID3DXBuffer *compilation_errors;
+    HRESULT hr;
+
+    /* Test binary effect */
+    compilation_errors = (ID3DXBuffer *)0xdeadbeef;
+    hr = D3DXCreateEffect(device, test_effect_variable_names_blob,
+            sizeof(test_effect_variable_names_blob), NULL, NULL, 0, NULL, &effect, &compilation_errors);
+    ok(hr == D3D_OK, "D3DXCreateEffect failed, got %#x, expected %#x\n", hr, D3D_OK);
+    ok(!compilation_errors, "Returned %p\n", compilation_errors);
+    effect->lpVtbl->Release(effect);
+
+    compilation_errors = (ID3DXBuffer *)0xdeadbeef;
+    hr = D3DXCreateEffect(NULL, NULL, 0, NULL, NULL, 0, NULL, NULL, &compilation_errors);
+    ok(hr == D3DERR_INVALIDCALL, "D3DXCreateEffect failed, got %#x, expected %#x\n", hr, D3DERR_INVALIDCALL);
+    ok(!compilation_errors, "Returned %p\n", compilation_errors);
+}
+
 START_TEST(effect)
 {
     HWND wnd;
@@ -2698,6 +2718,7 @@ START_TEST(effect)
     test_create_effect_compiler();
     test_effect_parameter_value(device);
     test_effect_variable_names(device);
+    test_effect_compilation_errors(device);
 
     count = IDirect3DDevice9_Release(device);
     ok(count == 0, "The device was not properly freed: refcount %u\n", count);