search

dokan-dev / dokany

User mode file system library for windows with FUSE Wrapper
http://dokan-dev.github.io
5.2k stars 661 forks source link

Dokan2: clarify breaking changes regarding FindFiles & FindFilesWithPattern #1084

Closed robert-j closed 2 years ago

robert-j commented 2 years ago

Environment

Check List

Description

The documentation of Dokany 2.0.3 FindFiles states that

  • \ref DOKAN_OPERATIONS.FindFilesWithPattern is checked first. If it is not implemented or
  • returns \c STATUS_NOT_IMPLEMENTED, then FindFiles is called, if implemented.

The documentation of Dokany 2.0.3 FindFilesWithPattern states that

  • If the function is not implemented, \ref DOKAN_OPERATIONS.FindFiles
  • will be called instead and the result will be filtered internally by the library.

This is not true in Dokany 2 any more. I've finished porting a project from 1.4 to 2.0.3 in which FindFiles* were implemented as follows:

static
NTSTATUS
DOKAN_CALLBACK
MyDriverFindFiles(
    LPCWSTR FileName,
    PFillFindData FillFindData,
    PDOKAN_FILE_INFO DokanFileInfo)
{
    return STATUS_NOT_IMPLEMENTED;
}

static
NTSTATUS
DOKAN_CALLBACK
MyDriverFindFilesWithPattern(
    LPCWSTR PathName,
    LPCWSTR SearchPattern,
    PFillFindData FillFindData,
    PDOKAN_FILE_INFO DokanFileInfo)
{
    // actual implementation
}

This code is causing all FindFirst operations to fail because Dokany is never invoking MyDriverFindFilesWithPattern. It is only invoking MyDriverFindFiles, and doesn't pay attention to STATUS_NOT_IMPLEMENTED.

I also tried to set DOKAN_OPERATIONS.FindFiles = NULL to no avail.

The document Update Dokan 1.1.0 application to Dokany 2.0.0 is also misleading as it states:

Previously FindFilesWithPattern could return STATUS_NOT_IMPLEMENTED and FindFiles would be called right after to process the request. This is no longer the case. If FindFilesWithPattern is assigned to the DOKAN_OPERATIONS struct, it will be called and STATUS_NOT_IMPLEMENTED will fail the request. So if FindFilesWithPattern is not implemented, it must not be assigned to DOKAN_OPERATIONS struct.

It must also mention that FindFiles must be implemented as well if FindFilesWithPattern is provided.

Liryna commented 2 years ago

Thanks @robert-j for raising this documentation issues!

I have updated the doc in the library to remove the FindFiles part https://github.com/dokan-dev/dokany/commit/54ad5d71dfed4f885d985ad52c2029a0067fd3f7 but I believe the FindFilesWithPattern is correct, if it is null assigned, FindFiles will be called. https://github.com/dokan-dev/dokany/blob/master/dokan/directory.c#L695-L714

It must also mention that FindFiles must be implemented as well if FindFilesWithPattern is provided.

We can improve this 👍 https://github.com/dokan-dev/dokany/commit/eb7b3ae4d6aeb4d02b7b2c3d0f852dad83793cf9

Does that look to be correct for you now ?

robert-j commented 2 years ago

@Liryna, yes, the patch looks good. Thank you.

The FindFilesWithPattern docs are indeed correct.

I believe that we should add nevertheless a note to FindFilesWithPattern's docs stating that IF a file system is implementing FindFilesWithPattern then it SHOULD implement FindFiles as well.

Liryna commented 2 years ago

That's true in the last release but with my last commit you can just implement FindFilesWithPattern and it will be called with a wildcard.

robert-j commented 2 years ago

Yes, I saw this. I was just nitpicking. SHOULD was meant as in "implement it when you can". It would save a lot of DokanIsNameInExpression calls, for example.

Liryna commented 2 years ago

Good point 👍 https://github.com/dokan-dev/dokany/commit/3cbd08ea8c08c1053d62d0e43a0011fe017e8f22 Note: The library code path used behind is the same whether FindFiles or FindFilesWithPattern was used during a non-pattern listing https://github.com/dokan-dev/dokany/blob/master/dokan/directory.c#L404

Thanks again for your report 😎