Re: [PATCH] ntdll: show/test the similarity of the first two outputparameters between RtlDosPathNameToNtPathName_U_WithStatus andRtlDosPathNameToRelativeNtPathName_U_WithStatus

[Nikolay Sivov]

On 04/13/2018 09:17 AM, Stefan Leichter wrote:
> - statusAbs = pRtlDosPathNameToNtPathName_U_WithStatus( path, &nameAbsW, &outpathAbsW, NULL );
> - statusRel = pRtlDosPathNameToRelativeNtPathName_U_WithStatus( path, &nameRelW, &outpathRelW, NULL );
> - ok(statusAbs == statusRel, "Got different results for path %s: %#x/%#x.\n", wine_dbgstr_w(path), statusAbs, statusRel);
> - if(statusAbs == statusRel) {
> -
>    ok(!lstrcmpW(nameAbsW.Buffer, nameRelW.Buffer), "for source %s nt names differ: %s/%s.\n",
> -
> 	wine_dbgstr_w(path), wine_dbgstr_w(nameAbsW.Buffer), wine_dbgstr_w(nameRelW.Buffer));
> -
> ok(!lstrcmpW(outpathAbsW, outpathRelW), "for source %s folders differ: %s/%s.\n",
> -
> 	wine_dbgstr_w(path), wine_dbgstr_w(outpathAbsW), wine_dbgstr_w(outpathRelW));
> - }
> - RtlFreeUnicodeString( &nameAbsW );
> - RtlFreeUnicodeString( &nameRelW );

This looks a bit broken. Matching statuses don't mean buffers are 
filled. What's a point of testing this together? I think it's more 
valuable to have two separate test functions, one for each.
> - if (!pRtlDosPathNameToRelativeNtPathName_U_WithStatus || !pRtlDosPathNameToNtPathName_U_WithStatus)
> - {
> -
>    win_skip("RtlDosPathNameToRelativeNtPathName_U_WithStatus() is %ssupported.\n",
> -
>             (pRtlDosPathNameToRelativeNtPathName_U_WithStatus ? "" : "not "));
> -
>    win_skip("RtlDosPathNameToNtPathName_U_WithStatus() is %ssupported.\n",
> -
>             (pRtlDosPathNameToNtPathName_U_WithStatus ? "" : "not "));
> -
>    return;
> - }

According to testbot results thoes are both available or both missing. 
Even if it was possible to have one and not another, printing skip 
message that says that something is supported is confusing. And again, 
can you test them both separately instead?