Re: [PATCH v11 1/2] ntdll: Introduce new nt_to_unix_file_name_with_case helper.

[Rémi Bernon]

On 5/20/21 5:35 PM, Gabriel Ivăncescu wrote:
> Which optionally returns the original case of the last path component.
> Signed-off-by: Gabriel Ivăncescu
>
> dlls/ntdll/unix/file.c | 66 +++++++++++++++++++++++++++---------------
>   1 file changed, 43 insertions(+), 23 deletions(-)
>
> diff --git a/dlls/ntdll/unix/file.c b/dlls/ntdll/unix/file.c
> index c033bb0..e649745 100644
> --- a/dlls/ntdll/unix/file.c
> +++ b/dlls/ntdll/unix/file.c
> @@ -3113,13 +3113,13 @@ done:
> - Helper for nt_to_unix_file_name
>
> */
>   static NTSTATUS lookup_unix_name( const WCHAR *name, int name_len, char **buffer, int unix_len, int pos,
> -
>                              UINT disposition, BOOL is_unix )
> -
>                              char **case_ret, UINT disposition, BOOL is_unix )
> {
>     static const WCHAR invalid_charsW[] = { INVALID_NT_CHARS, '/', 0 };
>     NTSTATUS status;
>     int ret;
>     struct stat st;
> - char *unix_name = *buffer;
> -
>
> char *unix_name = *buffer, file_case[MAX_DIR_ENTRY_LEN + 1];
>  const WCHAR *ptr, *end;
>
> /* check syntax of individual components */
>
> @@ -3157,6 +3157,12 @@ static NTSTATUS lookup_unix_name( const WCHAR *name, int name_len, char **buffer
>           for (p = unix_name + pos ; *p; p++) if (*p == '\') *p = '/';
>           if (!stat( unix_name, &st ))
>           {
> -
>        if (case_ret)
> -
>        {
> -
>            char *p = unix_name + pos + 1, *tmp = p;
> -
>            while (*p) if (*p++ == '/' && *p) tmp = p;
> -
>            *case_ret = strdup( tmp );
> -
>        }
>         if (disposition == FILE_CREATE) return STATUS_OBJECT_NAME_COLLISION;
>         return STATUS_SUCCESS;
>     }
>
> @@ -3169,6 +3175,7 @@ static NTSTATUS lookup_unix_name( const WCHAR *name, int name_len, char **buffer
>   /* now do it component by component */
>
> - file_case[0] = 0;
>  while (name_len)
>  {
>      const WCHAR *end, *next;
>
> @@ -3194,19 +3201,20 @@ static NTSTATUS lookup_unix_name( const WCHAR *name, int name_len, char **buffer
>           /* if this is the last element, not finding it is not necessarily fatal */
>           if (!name_len)
>           {
> -
>        ret = ntdll_wcstoumbs( name, end - name, file_case, MAX_DIR_ENTRY_LEN + 1, TRUE );
> -
>        if (ret < 0 || ret > MAX_DIR_ENTRY_LEN) ret = 0;
> -
>        file_case[ret] = 0;
> -
>         if (status == STATUS_OBJECT_PATH_NOT_FOUND)
>         {
>             status = STATUS_OBJECT_NAME_NOT_FOUND;
> -
>            if (disposition != FILE_OPEN && disposition != FILE_OVERWRITE)
> -
>            if (disposition != FILE_OPEN && disposition != FILE_OVERWRITE && ret)
>             {
> -
>                ret = ntdll_wcstoumbs( name, end - name, unix_name + pos + 1, MAX_DIR_ENTRY_LEN + 1, TRUE );
> -
>                if (ret > 0 && ret <= MAX_DIR_ENTRY_LEN)
> -
>                {
> -
>                    unix_name[pos] = '/';
> -
>                    unix_name[pos + 1 + ret] = 0;
> -
>                    status = STATUS_NO_SUCH_FILE;
> -
>                    break;
> -
>                }
> -
>                strcpy( unix_name + pos + 1, file_case );
> -
>                unix_name[pos] = '/';
> -
>                unix_name[pos + 1 + ret] = 0;
> -
>                status = STATUS_NO_SUCH_FILE;
> -
>                break;
>             }
>         }
>         else if (status == STATUS_SUCCESS && disposition == FILE_CREATE)
>
> @@ -3221,6 +3229,7 @@ static NTSTATUS lookup_unix_name( const WCHAR *name, int name_len, char **buffer
>           name = next;
>       }
> - if (case_ret) *case_ret = strdup( file_case );
>  return status;
> }

In addition to the comments I made on the previous version, I think this 
is also possibly leaking things in case_ret, depending on the status 
returned.

It may be better instead to take a bit of time considering a similar 
approach as for name_ret, where nt_to_unix_file_name(_no_root) would 
provide instead the case_ret / case_len buffer, returning it in the same 
way name_ret is.