[PATCH v2] kernel32: Ignore FileIoPriorityHintInfo return success

[Greg Smith]

There is no requirement to act on FileIoPriorityHintInfo
Return success when this is passed to SetFileInformationByHandle.

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=46130
Signed-off-by: Greg Smith 
---
v2: Updated test to move the file test before the file is marked
    for deletion, and reset last error before testing.
---
 dlls/kernel32/file.c       | 5 ++++-
 dlls/kernel32/tests/file.c | 8 ++++++++
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/dlls/kernel32/file.c b/dlls/kernel32/file.c
index eeccf67e15..80f4a2190d 100644
--- a/dlls/kernel32/file.c
+++ b/dlls/kernel32/file.c
@@ -1103,7 +1103,6 @@ BOOL WINAPI SetFileInformationByHandle( HANDLE file, FILE_INFO_BY_HANDLE_CLASS c
     case FileStreamInfo:
     case FileIdBothDirectoryInfo:
     case FileIdBothDirectoryRestartInfo:
-    case FileIoPriorityHintInfo:
     case FileFullDirectoryInfo:
     case FileFullDirectoryRestartInfo:
     case FileStorageInfo:
@@ -1119,6 +1118,10 @@ BOOL WINAPI SetFileInformationByHandle( HANDLE file, FILE_INFO_BY_HANDLE_CLASS c
         status = NtSetInformationFile( file, &io, info, size, FileDispositionInformation );
         break;

+    case FileIoPriorityHintInfo:
+        FIXME( "Ignoring file IO priority hint: %p, %u, %p, %u\n", file, class, info, size );
+        return TRUE;
+
     case FileStandardInfo:
     case FileCompressionInfo:
     case FileAttributeTagInfo:
diff --git a/dlls/kernel32/tests/file.c b/dlls/kernel32/tests/file.c
index 168bdd5e7b..4b95c95a7f 100644
--- a/dlls/kernel32/tests/file.c
+++ b/dlls/kernel32/tests/file.c
@@ -4914,6 +4914,7 @@ static void test_SetFileInformationByHandle(void)
     FILE_STANDARD_INFO stdinfo = { {{0}},{{0}},0,FALSE,FALSE };
     FILE_COMPRESSION_INFO compressinfo;
     FILE_DISPOSITION_INFO dispinfo;
+    FILE_IO_PRIORITY_HINT_INFO hintinfo;
     char tempFileName[MAX_PATH];
     char tempPath[MAX_PATH];
     HANDLE file;
@@ -4957,6 +4958,13 @@ static void test_SetFileInformationByHandle(void)
     ret = pSetFileInformationByHandle(file, FileRemoteProtocolInfo, &protinfo, sizeof(protinfo));
     ok(!ret && GetLastError() == ERROR_INVALID_PARAMETER, "got %d, error %d\n", ret, GetLastError());

+    /* FileIoPriorityHintInfo: Star Citizen installer fails when this call does not return sucess */
+    SetLastError(0xdeadbeef);
+    memset(&hintinfo,0,sizeof(hintinfo));
+    hintinfo.PriorityHint = IoPriorityHintNormal;
+    ret = pSetFileInformationByHandle(file, FileIoPriorityHintInfo, &hintinfo, sizeof(hintinfo));
+    ok(ret, "setting FileIoPriorityHintInfo got %d, error %d\n", ret, GetLastError());
+
     /* test FileDispositionInfo, additional details already covered by ntdll tests */
     SetLastError(0xdeadbeef);
     ret = pSetFileInformationByHandle(file, FileDispositionInfo, &dispinfo, 0);

[Marvin]

Hi,

While running your changed tests on Windows, I think I found new failures.
Being a bot and all I'm not very good at pattern recognition, so I might be
wrong, but could you please double-check?

Full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=44497

Your paranoid android.

=== wvistau64 (64 bit report) ===

kernel32:
file.c:4966: Test failed: setting FileIoPriorityHintInfo got 0, error 998

=== w2008s64 (64 bit report) ===

kernel32:
file.c:4966: Test failed: setting FileIoPriorityHintInfo got 0, error 998

=== w7pro64 (64 bit report) ===

kernel32:
file.c:4966: Test failed: setting FileIoPriorityHintInfo got 0, error 998

=== w864 (64 bit report) ===

kernel32:
file.c:4966: Test failed: setting FileIoPriorityHintInfo got 0, error 998

=== w1064 (64 bit report) ===

kernel32:
file.c:4966: Test failed: setting FileIoPriorityHintInfo got 0, error 998

=== debian9 (32 bit report) ===

kernel32:
change.c:318: Test failed: should be ready

[Dmitry Timoshkov]

Greg Smith  wrote:
> - case FileIoPriorityHintInfo:
> -
>    FIXME( "Ignoring file IO priority hint: %p, %u, %p, %u\n", file, class, info, size );
> -
>    return TRUE;

It would be helpful to add standard parameter checks.

[Zebediah Figura]

On 11/16/18 10:40 PM, Dmitry Timoshkov wrote:
> Greg Smith  wrote:
> > - case FileIoPriorityHintInfo:
> > -
> >    FIXME( "Ignoring file IO priority hint: %p, %u, %p, %u\n", file, class, info, size );
> > -
> >    return TRUE;
>
> It would be helpful to add standard parameter checks.

It may also be helpful to forward it to ntdll and implement the stub 
there, though perhaps this isn't worthwhile.

[Greg]

On 17/11/18 3:46 pm, Zebediah Figura wrote:
> On 11/16/18 10:40 PM, Dmitry Timoshkov wrote:
> > Greg Smith  wrote:
> > > +    case FileIoPriorityHintInfo:
> > > +        FIXME( "Ignoring file IO priority hint: %p, %u, %p, %u\n", 
> > > file, class, info, size );
> > > +        return TRUE;
> >
> > It would be helpful to add standard parameter checks.
>
> It may also be helpful to forward it to ntdll and implement the stub 
> there, though perhaps this isn't worthwhile.

This patch is an alteration to SetFileInformationByHandle which already 
exists in kernel32/file.c (line 1089) and currently "pretty much" only 
supports FileDispositionInfo. Are you suggesting I should move the whole 
method to ntdll? Or just create a stub there for when 
SetFileInformationByHandle is called with this particular class 
parameter value?

Thanks for the feedback - this is my first patch, so I appreciate the 
guidance.

[Greg]

On 17/11/18 3:40 pm, Dmitry Timoshkov wrote:
> Greg Smith  wrote:
> > - case FileIoPriorityHintInfo:
> > -
> >    FIXME( "Ignoring file IO priority hint: %p, %u, %p, %u\n", file, class, info, size );
> > -
> >    return TRUE;
>
> It would be helpful to add standard parameter checks.

By standard parameter checks, given that class is effectively checked by 
the switch, I take that you mean:
- Verify the handle is not invalid;
- NULL pointer check on info; and
- size is consistent with FILE_IO_PRIORITY_HINT_INFO?

Thanks for the feedback - this is my first patch, so I appreciate the 
guidance.

[Dmitry Timoshkov]

Greg  wrote:
> > > - case FileIoPriorityHintInfo:
> > > -
> > >    FIXME( "Ignoring file IO priority hint: %p, %u, %p, %u\n", file, class, info, size );
> > > -
> > >    return TRUE;
> >
> > It would be helpful to add standard parameter checks.
>
> By standard parameter checks, given that class is effectively checked by 
> the switch, I take that you mean:
> - Verify the handle is not invalid;
> - NULL pointer check on info; and
> - size is consistent with FILE_IO_PRIORITY_HINT_INFO?

Yes, that's what I meant with 'standard parameter checks'.