Future changes (TODO)

[Liryna]

For fixing some issue and becoming more stable, Dokan need

• KeepAlive no longer a option.
• Use NTSTATUS for interface functions - For now: LONG = NTSTATUS * -1 - Will break compatiblity
• Use LookasideList
• Merge DriverEntry (catch all exception in one place like cdfs) Irp pending timeout issue #55
• SectionObjectPointers have the information about Memory Manager and if the file is memory mapped. But we need to implement it in a clean way. see Possible race conditions in mirror.c sample. #32
• Add Oplock Semantics
• Change EnumerateNamedStreams to work like FindFiles
• Change Dokan Mount Service by Microsoft Mount Manager (requires Plug & Play) see Mount Manager #50
• Read-only device option Read-only device option #72
• CreateFile SL_CASE_SENSITIVE, SL_FORCE_ACCESS_CHECK, SL_OPEN_PAGING_FILE are not currently handled at all. CreateFile cleanup #99
• Add option for normal user to mount device as Global
• Disable network device when mount point folder is specified

[Liryna]

Hi @marinkobabic,
I am currently changing NTSTATUS as plained. (https://github.com/dokan-dev/dokany/compare/NTSTATUS)
I am facing a issue with CreateFile that it seems you faced too 😃

###Create 0000
   CreateDisposition 0x00000001
CreateFile : C:\Users\autorun.inf
  AccountName: Adrien, DomainName: WIN-9RM07VT4S97
        OPEN_EXISTING
        ShareMode = 0x7
        FILE_SHARE_READ
        FILE_SHARE_WRITE
        FILE_SHARE_DELETE
        AccessMode = 0x80
        FILE_READ_ATTRIBUTES
        FlagsAndAttributes = 0x0
        error code = 2

CreateFile status = 2
error openInfo is NULL
###GetFileInfo -001
GetFileInfo : C:\Users\autorun.inf
        invalid handle
**MIRROR CRASH**

This happened since in mirror I return the error from getLastError.
https://github.com/dokan-dev/dokany/blob/NTSTATUS/dokan_mirror/mirror.c#L220

error openInfo is NULL happen here
https://github.com/dokan-dev/dokany/blob/NTSTATUS/dokan/dokan.c#L487
because openInfo is free here
https://github.com/dokan-dev/dokany/blob/NTSTATUS/dokan/create.c#L212

By removing the free like you, I was surprised to have no crash.
marinkobabic/dokanx@eeb692a#diff-d7735925422d2f0b4a057d4f1ec7a0d5R261

It seems that dokanx have chosen to fix it in a different way...
https://github.com/BenjaminKim/dokanx/blob/develop/mirrorfs/mirrorfs.cpp#L227

By the way, the comment say that Close is never called...but I get a Cleanup and close after GetFileInfo.

Do you have a reason for removing this lines ? Are they important ?

Otherwise mirror is correctly running without problem. I will run dokan-net.Test before merging it.

[marinkobabic]

@Liryna
The comment says "Needs to free openInfo because Close is never called." The developer expected that the close will not be executed because of the status pEventInfo->Status != STATUS_SUCCESS

The fact is, that even in this case the cleanup is executed. Therefore the pOpenInfo should not be freed during create. Cleanup requires the pOpenInfo.

Here you have the mapping table Win32 to NTSTATUS https://support.microsoft.com/en-us/kb/113996

GetLastError code should not be passed directly to the driver. GetLastError is the Win32 error and not NTSTATUS.

[Liryna]

Thank you for the clear answer 👍

Do you think we should let the user return Win32 error and we convert it in NTSTATUS (dokan.dll)?

[marinkobabic]

No. Then you will have always a discussion of mapping. Which win32 errors do you map to which code. Please add this code as well and so on...

[Liryna]

Ok ! I agree 😃
I remove the free and continue the changes.

[Liryna]

@marinkobabic, I finished the NTSTATUS changes. I tested it with the mirror and it seems to work good / without crash.

I would like your review before merging it 😃 and know if it fit your request ! (and If you have the time ofc)
https://github.com/dokan-dev/dokany/compare/NTSTATUS

[marinkobabic]

@Liryna
Wow, a lot of changes. You had also to remove a lot of code 😄 Great job! 👍

• Please check your code for return 0 and replace it using the STATUS_SUCCESS.
• Also check where result == 0 is used and change the code if possible.
• In generally is the question if status > 0 should be used. Inside of the driver NT_SUCCESS and NT_ERROR is used http://rogerkar.blogspot.ch/2009/06/nterror-ntsuccess-identifier-not-found.html. Here the definition https://msdn.microsoft.com/en-us/library/windows/hardware/ff565436(v=vs.85).aspx

[Liryna]

Thank @marinkobabic for your feedback and informations !
I made the changes as you suggested !

I will merge it in master when I finish to change dokan-net with NtStatus.

[Liryna]

The todo has been split to different tickets.