FarGroup / FarManager

File and Archive Manager
https://farmanager.com
BSD 3-Clause "New" or "Revised" License
1.83k stars 205 forks source link

Copy file bigger than target file system can store #532

Open pmisik opened 2 years ago

pmisik commented 2 years ago

Far Manager version

3.0.5959 x64

OS version

Microsoft Windows [Version 10.0.19044.1766]

Other software

No response

Steps to reproduce

Prerequisites:

  1. Source file to copy on extFAT/NTFS file system(FS) with size bigger than target file system FAT32 can handle (I noticed problem while trying to copy file with size 4 628 822 638 bytes)
  2. Destination FS(FAT32) with enough free space to copy file but it exceeds maximum file size on target FS

Far settings

Options > System settings > Use system copy routine

  1. On
  2. Off

Step to reproduce

Hit F5 Copy on prerequisite file with default settings from extFAT/NTFS FS to FAT32

Expected behavior

In both cases _Far settings 1+2_ report better error message like Windows Explorer does The file '%s' is too large for the destination file system.

In case Far settings 2(Use system copy routine is off) be clever and do not try to copy file at all that leads into incorrect/misleading error message There is not enough space on the disk and report better error message The file '%s' is too large for the destination file system.

Actual behavior

Windows Explorer report more useful message in this case before it attempt to copy file image

alabuzhev commented 2 years ago

The parameter is incorrect

Unfortunately, this is exactly what the system copy function returns. You can see the same error message from the "copy" command in cmd.

There is not enough space on the disk

Unfortunately, this is exactly what the write function returns when file size hits the limit.

Far tries to copy file

This one is actually bad - we shouldn't even try to copy it if we failed to reserve the space for it. Should be fixed in da23441.

It would be nice to improve these error messages indeed, but so far it looks like doing so would need either switching to Shell APIs for file copying (like Explorer), which is not realistic, or coming up with our own heuristics.

pmisik commented 2 years ago

Unfortunately, this is exactly what the system copy function returns. You can see the same error message from the "copy" command in cmd.

Did I correctly find out, in case Use system copy routine is on, it is called CopyFileEx from https://github.com/FarGroup/FarManager/blob/master/far/platform.fs.cpp#L1524?

I created simple program using CopyFileEx and also ""modern"" CopyFile2 and both cases GetLastError returns ERROR_INVALID_PARAMETER - exactly as you replied in previous comment.

It would be nice to improve these error messages indeed, but so far it looks like doing so would need either switching to Shell APIs for file copying (like Explorer), which is not realistic, or coming up with our own heuristics.

Could you be more specific which function from Shell APIs you talk about? In Import address table of explorer.exe and I could not find any specific function related to file copying. There is chance it is imported by Ordinal or dynamically at runtime by GetProcAddress. Surprisingly I see it is imported CopyFileW.

alabuzhev commented 2 years ago

Did I correctly find out, in case Use system copy routine is on, it is called CopyFileEx from https://github.com/FarGroup/FarManager/blob/master/far/platform.fs.cpp#L1524?

Correct.

Could you be more specific which function from Shell APIs you talk about?

I have no idea tbh. I just looked for the string from your screenshot ("too large for the destination file system") in my Windows directory and found it in System32\en-US\shell32.dll.mui, so the implementation is probably somewhere in shell32.dll.

There is chance it is imported by Ordinal or dynamically at runtime by GetProcAddress.

I suspect it's not a plain function, but IFileOperation or something like that.