[PATCH v5] ntdll/directory: Add support for EXT4 case folding per directory

[Huw Davies]

ntdll/directory: -> ntdll:

On Thu, Jun 13, 2019 at 02:44:58PM +0300, Gabriel Ivăncescu wrote:
> This adds support for checking the case-insensitive attribute on ext4
> with newer kernels so that Wine can rely on it for performance. Checking
> for the EXT4_CASEFOLD_FL attribute involves an ioctl, which operates on
> file descriptors, while all the former checks operated directly on the
> dir pathname (e.g. statfs). Obviously, it's best to avoid looking up the
> directory multiple times (also for correctness, so it refers to the same
> dir). So in the case that we *do* have a file descriptor, then use it
> everywhere, with e.g. fstatfs.

This could be re-worded a bit.  In particular mentioning the file descriptor
change is unnecessary.

> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=47099
> Signed-off-by: Gabriel Ivăncescu <gabrielopcode at gmail.com>
> ---
> 
> v5: Only use the file descriptor on Linux. The second patch is not resent
> since it's in staging for users to test out first before going upstream.
> 
>  dlls/ntdll/directory.c | 44 +++++++++++++++++++++++++++---------------
>  1 file changed, 28 insertions(+), 16 deletions(-)
> 
> diff --git a/dlls/ntdll/directory.c b/dlls/ntdll/directory.c
> index bbdbbe9..9b4e61b 100644
> --- a/dlls/ntdll/directory.c
> +++ b/dlls/ntdll/directory.c
> @@ -115,6 +115,8 @@ WINE_DEFAULT_DEBUG_CHANNEL(file);
>  
>  /* just in case... */
>  #undef VFAT_IOCTL_READDIR_BOTH
> +#undef EXT2_IOC_GETFLAGS
> +#undef EXT4_CASEFOLD_FL
>  
>  #ifdef linux
>  
> @@ -130,6 +132,12 @@ typedef struct
>  /* Define the VFAT ioctl to get both short and long file names */
>  #define VFAT_IOCTL_READDIR_BOTH  _IOR('r', 1, KERNEL_DIRENT [2] )
>  
> +/* Define the ext2 ioctl for handling extra attributes */
> +#define EXT2_IOC_GETFLAGS _IOR('f', 1, long)
> +
> +/* Case-insensitivity attribute */
> +#define EXT4_CASEFOLD_FL 0x40000000
> +
>  #ifndef O_DIRECTORY
>  # define O_DIRECTORY 0200000 /* must be directory */
>  #endif
> @@ -1113,7 +1121,7 @@ static int get_dir_case_sensitivity_attr( const char *dir )
>   *           get_dir_case_sensitivity_stat
>   *
>   * Checks if the volume containing the specified directory is case
> - * sensitive or not. Uses statfs(2) or statvfs(2).
> + * sensitive or not. Uses (f)statfs(2), statvfs(2), fstatat(2), or ioctl(2).
>   */
>  static BOOLEAN get_dir_case_sensitivity_stat( const char *dir )
>  {
> @@ -1165,14 +1173,23 @@ static BOOLEAN get_dir_case_sensitivity_stat( const char *dir )
>      return FALSE;
>  
>  #elif defined(__linux__)
> +    BOOLEAN sens = FALSE;
>      struct statfs stfs;
>      struct stat st;
> -    char *cifile;
> +    int fd, flags;
> +
> +    if ((fd = open( dir, O_RDONLY | O_NONBLOCK | O_LARGEFILE )) == -1)
> +        return FALSE;

I think returning TRUE in an error case here is the correct thing to do.
TRUE is after all the default.

> +
> +    if (ioctl( fd, EXT2_IOC_GETFLAGS, &flags ) != -1 && (flags & EXT4_CASEFOLD_FL))
> +        goto end;
>  
>      /* Only assume CIOPFS is case insensitive. */
> -    if (statfs( dir, &stfs ) == -1) return FALSE;
> +    if (fstatfs( fd, &stfs ) == -1) goto end;
> +
> +    sens = TRUE;
>      if (stfs.f_type != 0x65735546 /* FUSE_SUPER_MAGIC */)
> -        return TRUE;
> +        goto end;
>      /* Normally, we'd have to parse the mtab to find out exactly what
>       * kind of FUSE FS this is. But, someone on wine-devel suggested
>       * a shortcut. We'll stat a special file in the directory. If it's
> @@ -1180,17 +1197,12 @@ static BOOLEAN get_dir_case_sensitivity_stat( const char *dir )
>       * This will break if somebody puts a file named ".ciopfs" in a non-
>       * CIOPFS directory.
>       */
> -    cifile = RtlAllocateHeap( GetProcessHeap(), 0, strlen( dir )+sizeof("/.ciopfs") );
> -    if (!cifile) return TRUE;
> -    strcpy( cifile, dir );
> -    strcat( cifile, "/.ciopfs" );
> -    if (stat( cifile, &st ) == 0)
> -    {
> -        RtlFreeHeap( GetProcessHeap(), 0, cifile );
> -        return FALSE;
> -    }
> -    RtlFreeHeap( GetProcessHeap(), 0, cifile );
> -    return TRUE;
> +    if (fstatat( fd, ".ciopfs", &st, AT_NO_AUTOMOUNT ) == 0)
> +        sens = FALSE;
> +
> +end:
> +    close( fd );
> +    return sens;

This could all be re-written without the gotos which would make it much clearer.
Something like:

    BOOLEAN sens = TRUE;

    if ((fd = open( dir, O_RDONLY | O_NONBLOCK | O_LARGEFILE )) == -1)
        return TRUE;

    if (ioctl( fd, EXT2_IOC_GETFLAGS, &flags ) != -1 && (flags & EXT4_CASEFOLD_FL))
    {
        sens = FALSE;
    }
    else
    {
        /* Long comment about some chat on wine-devel... */
        if ((fstatfs( fd, &stfs ) != -1) && (stfs.f_type == 0x65735546 /* FUSE_SUPER_MAGIC */) &&
            (fstatat( fd, ".ciopfs", &st, AT_NO_AUTOMOUNT ) == 0))
            sens = FALSE;
    }
    close( fd );
    return sens;