Re: [PATCH] fs/ntfs3: Fix logic in ntfs_cmp_names_cpu

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 06.10.2021 20:03, Mark Harmstone wrote:
> On 6/10/21 15:37, Konstantin Komarov wrote:
>> So it's expected for `ls WINDOWS` to fail.
> 
> I'm confused. If this deliberate, why does cmp_fnames pass the upcase pointer
> to ntfs_cmp_names_cpu? Or for that matter, why do you bother loading the
> upcase block at all?
> 

As I've said, it's expected, that ntfs3 case sensitive.
So it's expected for `ls WINDOWS` to fail.

About upcase block. How ntfs_cmp_names_cpu work:
First we compare case sensitive.
If we found something ( diff1 ), then we go to case insensitive loop
( that's where upcase is needed ).
There we compare case insensitive.
If we found something ( diff2 ), then we return it.
If not, then we return case sensitive result ( diff1 ).

Why such convoluted process?
This is optimized way of this algorithm ( from comment to ntfs_cmp_names ):
 * Straight way to compare names:
 * - Case insensitive
 * - If name equals and 'upcase' then
 * - Case sensitive

>> The same behavior you can see in
>> CreateFileA function with FILE_FLAG_POSIX_SEMANTICS [1]
>> or in NtSetInformationFile function with FileCaseSensitiveInformation [2].
> 
> Two different things. AIUI, FILE_FLAG_POSIX_SEMANTICS passes the
> SL_CASE_SENSITIVE flag through to the FS driver's IRP_MJ_CREATE function, doing
> a case-sensitive lookup on what would otherwise be a normal directory. The
> FileCaseSensitiveInformation flag is set on the inode and forces Windows 10 to
> always do case-sensitive lookups for that directory (i.e. it's the inverse of
> ext4's casefolding flag).
> 

Two different things from perspective of implementation.
>From perspective of user they are similar: case sensitive lookup.

> This issue came to my attention because I've been doing a lot of research into
> NTFS recently, and want to add FileCaseSensitiveInformation support to your
> driver. But obviously this issue is a blocker for that.
> 
>> We must always compare filenames with bothcase == true.
> 
> You *really* need to rename this variable, it makes no sense at it is. As I
> said, as far as I can make out it means "assume that one of the strings is
> already uppercase".
> 

Yes, that's what I've meant by "There will be patch to fix this situation".
Patch fixes situation with bothcase variable.

>> There will be patch to fix this situation.
> 
> Not to be funny, but I've already provided you with one... You'll drive away
> potential contributors if you ignore their patches without a good reason.

As I've said previously right now ntfs3 is case sensitive.
Your patch will change it ( case sensitive result diff1 won't be returned ).
I've opened discussion on theme "should ntfs3 be case sensitive or case
insensitive" by writing "Should ntfs3 work with `ls WINDOWS`?".
I think, that ntfs3 need to be case sensitive.
And listed things, that I think support my opinion.
That's why I've mentioned that ntfs.sys is case sensitive.
That's why I've mentioned FILE_FLAG_POSIX_SEMANTICS and
FileCaseSensitiveInformation.
I don't think, that copying win32 api behavior is good,
when Microsoft makes steps to make ntfs work in posix way.
I'm sorry for not writing it more clear in previous letter.

Please don't think, that your work is disregarded.
You already found problem with bothcase.
I just can't accept patch, that will change default behavior of driver from
"case sensitive" to "case insensitive", without any discussion.

If I remember correctly, to make ntfs3 case insensitive you will need to provide
special functions d_hash and d_compare from struct dentry_operations
( examples: fs/fat and fs/exfat ).
Right now we use default versions, because they work with case sensitive fs.




[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux