[PATCH v3 1/2] ntdll/directory: Add support for EXT4 case folding per directory

[Gabriel Ivăncescu]

Hi Huw,

On 6/7/19 12:47 PM, Huw Davies wrote:
> On Fri, May 24, 2019 at 03:09:34PM +0300, Gabriel Ivăncescu wrote:
> 
> I'm not sure we need to define these ourselves, we can
> just pull them from the headers.  I realise that you're
> copying what was done with VFAT_IOCTL_READDIR_BOTH and
> I don't know why that was done that way, but unless
> there's a reason to do this, let's not add more.
> 

EXT2_IOC_GETFLAGS seems to be defined in linux/ext2_fs.h. The problem is 
that I don't seem to have that header neither on my machine or build 
environments. I'm guessing it's part of some different dependency. (of 
course, I can use the ioctl manually if I define it myself)

It's probably the reason VFAT_IOCTL_READDIR_BOTH was defined like that 
as well, I'm guessing most people or distros don't have these headers 
with just Wine's dependencies.

Should I still import it from header or leave it as is?

>> +/* Use a file descriptor for the case-sensitivity check if we have to */
>> +#if defined(EXT2_IOC_GETFLAGS) && defined(EXT4_CASEFOLD_FL)
>> +#define GET_DIR_CASE_SENSITIVITY_USE_FD 1
>> +#else
>> +#define GET_DIR_CASE_SENSITIVITY_USE_FD 0
>> +#endif
> 
> This is now just used in one place, so get rid of it use the
> preprocessor condition at the relevant place below.
> 
>>   static BOOLEAN get_dir_case_sensitivity( const char *dir )
>>   {
>> +    int case_sensitive, fd = -1;
>> +
>>   #if defined(HAVE_GETATTRLIST) && defined(ATTR_VOL_CAPABILITIES) && \
>>       defined(VOL_CAPABILITIES_FORMAT) && defined(VOL_CAP_FMT_CASE_SENSITIVE)
>> -    int case_sensitive = get_dir_case_sensitivity_attr( dir );
>> +    case_sensitive = get_dir_case_sensitivity_attr( dir );
>>       if (case_sensitive != -1) return case_sensitive;
>>   #endif
>> -    return get_dir_case_sensitivity_stat( dir );
>> +
>> +    if (GET_DIR_CASE_SENSITIVITY_USE_FD)
> 
> #if defined(EXT2_IOC_GETFLAGS) && defined(EXT4_CASEFOLD_FL)
> 
> No need for the if block.
> 

The problem is that the preprocessor would eliminate the code from 
parsing, so when it's not defined, there will be warnings about unused 
variables or unused labels (the end label). That's why I used an if 
statement instead.

I can of course do something like:

#if !defined(EXT2_IOC_GETFLAGS) || !defined(EXT4_CASEFOLD_FL)
if (0)
#endif
{
     ...
}

but I'm not sure that's preferable since it looks ugly.


An alternative would be to use a helper function to open the fd, and in 
case we don't have the #defines, we just return -1 from it. Would that 
be suitable?