[PATCH] advapi32: succeed for null pointer in ChangeServiceConfig2

Nikolay Sivov nsivov at codeweavers.com
Mon Mar 23 15:05:44 CDT 2015


On 03/23/2015 10:50 PM, Bernhard Übelacker wrote:
> Hello Nikolay, hello Stefan,
 > ...
> Would this patch for the test side be acceptable?
> https://testbot.winehq.org/JobDetails.pl?Key=12322


> +    if(!pChangeServiceConfig2W)
> +    {
> +        win_skip("function ChangeServiceConfig2W not present\n");
> +        goto cleanup;
> +    }

Is it really possible to have ChangeServiceConfig2A() and not 
ChangeServiceConfig2W()? If those are both present on win2k then you 
don't need a skip.

> +    SetLastError(0xdeadbeef);
> +    pConfigW->lpDescription = (LPWSTR)descriptionW;
> +    ret = pChangeServiceConfig2W(svc_handle, SERVICE_CONFIG_DESCRIPTION, &buffer);
> +    ok(ret, "expected ChangeServiceConfig2W to succeed\n");

No need to set last error in this case.

> +    SetLastError(0xdeadbeef);
> +    pConfigW->lpDescription = 0xdeadbeef;
> +    ret = pQueryServiceConfig2W(svc_handle, SERVICE_CONFIG_DESCRIPTION,buffer,sizeof(buffer),&needed);
> +    ok(ret, "expected QueryServiceConfig2A to succeed\n");
> +    ok(pConfigW->lpDescription && !lstrcmpW(descriptionW,pConfigW->lpDescription),
> +        "expected lpDescription to be %s, got %s\n",wine_dbgstr_w(descriptionW) ,wine_dbgstr_w(pConfigW->lpDescription));

Same here, also it's enough to set description to NULL, as you expect it 
not to be NULL on return.

> +    SetLastError(0xdeadbeef);
> +    pConfigW->lpDescription = 0;
> +    ret = pChangeServiceConfig2W(svc_handle, SERVICE_CONFIG_DESCRIPTION, &buffer);
> +    todo_wine
> +    {
> +    ok(ret, "expected ChangeServiceConfig2W to succeed\n");
> +    }

Again, no need to set last error.

So according to test setting NULL description does not fail, but doesn't 
change description either. Could you add another test with invalid 
service handle and NULL description to see if it fails with invalid 
handle error?

If it does fail with such error then you need to fix a server part.

>
> And should the test be switched to the *A functions?
> https://testbot.winehq.org/JobDetails.pl?Key=12374

I think we need both. Especially since Wine forwards A->W at client.




More information about the wine-devel mailing list