Re: [PATCH 2/2] shell32/tests: Check IDispatch for NULL before using its functions

[Nikolay Sivov]

On 25.09.2017 21:34, Fabian Maurer wrote:
> Fixes Bug 43749
> Signed-off-by: Fabian Maurer
>
> dlls/shell32/tests/shelldispatch.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/dlls/shell32/tests/shelldispatch.c b/dlls/shell32/tests/shelldispatch.c
> index 7b5afa0c94..b31497b557 100644
> --- a/dlls/shell32/tests/shelldispatch.c
> +++ b/dlls/shell32/tests/shelldispatch.c
> @@ -134,13 +134,15 @@ static void test_namespace(void)
>      r = IShellDispatch_get_Application(sd, &disp);
>      ok(r == S_OK, "Failed to get application pointer, hr %#x.\n", r);
>      ok(disp == (IDispatch *)sd, "Unexpected application pointer %p.\n", disp);
> - IDispatch_Release(disp);
> -
>
> if(disp)
> -
>    IDispatch_Release(disp);
>
> disp = NULL;
>    r = IShellDispatch_get_Parent(sd, &disp);
>    ok(r == S_OK, "Failed to get Shell object parent, hr %#x.\n", r);
>    ok(disp == (IDispatch *)sd, "Unexpected parent pointer %p.\n", disp);
> - IDispatch_Release(disp);
> -
>
> if(disp)
> -
>    IDispatch_Release(disp);
>
> VariantInit(&var);
>    folder = (void*)0xdeadbeef;
>
> @@ -502,7 +504,8 @@ static void test_items(void)
>      r = FolderItem_get_Parent(item, &disp);
>      ok(r == S_OK, "Failed to get parent pointer, hr %#x.\n", r);
>      ok(disp == (IDispatch *)folder, "Unexpected parent pointer %p.\n", disp);
> - IDispatch_Release(disp);
> -
>
> if(disp)
> -
>    IDispatch_Release(disp);
>
> if (item) FolderItem_Release(item);
>    VariantClear(&var);
>
> @@ -1036,6 +1039,10 @@ todo_wine {
>          ok(disp == NULL, "got %p\n", disp);
>          ok(ret == 0, "got %d\n", ret);
>      }
> - else if (disp == NULL) {
> -
>    ok(FALSE, "disp is NULL\n");
> -
>    skip("disp is NULL\n");
> - }
> else {
>     static const IID *browser_riids[] = {
>         &IID_IWebBrowser2,
>
> @@ -1055,7 +1062,6 @@ todo_wine {
>          IShellView *sv;
>          IUnknown *unk;
> -
>    ok(disp != NULL, "got %p\n", disp);
>    ok(ret != HandleToUlong(hwnd), "got %d\n", ret);
>
>    /* IDispatch-related tests */

So what happens exactly that makes get_Parent() and get_Application()
fail? I don't think workarounds like that are useful without
understanding the problem.

[PATCH 2/2] shell32/tests: Check IDispatch for NULL before using its functions

[Fabian Maurer]

On Montag, 25. September 2017 20:44:26 CEST Nikolay Sivov wrote:
> So what happens exactly that makes get_Parent() and get_Application()
> fail? I don't think workarounds like that are useful without
> understanding the problem.

I don't know, I just figured it'll make sense for the tests not to crash 
regardless.

Regards,
Fabian Maurer