[PATCH 01/11] shell32: _SHCreateSymbolicLinks() remove directory test for XDG Documents folder

Bob Wya bob.mt.wya at gmail.com
Wed Jul 18 13:18:33 CDT 2018


On 16 July 2018 at 23:46, Rob Walker <bob.mt.wya at gmail.com> wrote:

> XDG_UserDirLookup() will return a NULL result for any XDG directory path
> that is
> not a valid directory or is a broken symlinked directory path.
> So we can remove a redundant test, that ensures the returned
> XDG_DOCUMENTS_DIR
> path is a valid directory.
> Tidy up the inline code comments, removing some that are unnecessary.
>
> Signed-off-by: Rob Walker <bob.mt.wya at gmail.com>
> ---
>  dlls/shell32/shellpath.c | 35 +++++++++++++++--------------------
>  1 file changed, 15 insertions(+), 20 deletions(-)
>
> diff --git a/dlls/shell32/shellpath.c b/dlls/shell32/shellpath.c
> index de4bf7d566..0d8a5987d7 100644
> --- a/dlls/shell32/shellpath.c
> +++ b/dlls/shell32/shellpath.c
> @@ -4424,7 +4424,7 @@ static void _SHCreateSymbolicLinks(void)
>      char ** xdg_results;
>      char * xdg_desktop_dir;
>
> -    /* Create all necessary profile sub-dirs up to 'My Documents' and get
> the unix path. */
> +    /* Create the '%USERPROFILE%\\My Documents' directory path. */
>      hr = SHGetFolderPathW(NULL, CSIDL_PERSONAL|CSIDL_FLAG_CREATE, NULL,
>                            SHGFP_TYPE_DEFAULT, wszTempPath);
>      if (FAILED(hr)) return;
> @@ -4439,7 +4439,7 @@ static void _SHCreateSymbolicLinks(void)
>      {
>          while (1)
>          {
> -            /* Check if there's already a Wine-specific 'My Documents'
> folder */
> +            /* Try to target a pre-existing '$HOME/My Documents' folder.
> */
>              strcpy(szPersonalTarget, pszHome);
>              if (_SHAppendToUnixPath(szPersonalTarget,
> MAKEINTRESOURCEW(IDS_PERSONAL)) &&
>                  !stat(szPersonalTarget, &statFolder) &&
> S_ISDIR(statFolder.st_mode))
> @@ -4457,28 +4457,23 @@ static void _SHCreateSymbolicLinks(void)
>                  break;
>              }
>
> -            /* Try to point to the XDG Documents folder */
> -            if (xdg_results && xdg_results[num-2] &&
> -               !stat(xdg_results[num-2], &statFolder) &&
> -               S_ISDIR(statFolder.st_mode))
> +            /* Try to target the XDG_DOCUMENTS_DIR folder. */
> +            if (xdg_results && xdg_results[num-2])
>              {
>                  strcpy(szPersonalTarget, xdg_results[num-2]);
>                  break;
>              }
>
> -            /* Or the hardcoded / OS X Documents folder */
> +            /* Try to target the hardcoded / OS X 'Documents' folder. */
>              strcpy(szPersonalTarget, pszHome);
>              if (_SHAppendToUnixPath(szPersonalTarget, DocumentsW) &&
> -               !stat(szPersonalTarget, &statFolder) &&
> -               S_ISDIR(statFolder.st_mode))
> +                !stat(szPersonalTarget, &statFolder) &&
> S_ISDIR(statFolder.st_mode))
>                  break;
>
> -            /* As a last resort point to $HOME. */
> +            /* Target the '$HOME' folder directly (fallback). */
>              strcpy(szPersonalTarget, pszHome);
>              break;
>          }
> -
> -        /* Replace 'My Documents' directory with a symlink or fail
> silently if not empty. */
>          remove(pszPersonal);
>          symlink(szPersonalTarget, pszPersonal);
>      }
> @@ -4498,30 +4493,30 @@ static void _SHCreateSymbolicLinks(void)
>      /* Create symbolic links for 'My Pictures', 'My Videos' and 'My
> Music'. */
>      for (i=0; i < ARRAY_SIZE(aidsMyStuff); i++)
>      {
> -        /* Create the current 'My Whatever' folder and get its unix path.
> */
> +        /* Create the '%USERPROFILE%\\My XXX' directory path. */
>          hr = SHGetFolderPathW(NULL, acsidlMyStuff[i]|CSIDL_FLAG_CREATE,
> NULL,
>                                SHGFP_TYPE_DEFAULT, wszTempPath);
>          if (FAILED(hr)) continue;
>
>          pszMyStuff = wine_get_unix_file_name(wszTempPath);
>          if (!pszMyStuff) continue;
> -
> +
>          while (1)
>          {
> -            /* Check for the Wine-specific '$HOME/My Documents' subfolder
> */
> +            /* Try to target a pre-existing '$HOME/My Documents/My XXX'
> folder. */
>              strcpy(szMyStuffTarget, szPersonalTarget);
>              if (_SHAppendToUnixPath(szMyStuffTarget,
> MAKEINTRESOURCEW(aidsMyStuff[i])) &&
>                  !stat(szMyStuffTarget, &statFolder) &&
> S_ISDIR(statFolder.st_mode))
>                  break;
>
> -            /* Try the XDG_XXX_DIR folder */
> +            /* Try to target the XDG_XXX_DIR folder. */
>              if (xdg_results && xdg_results[i])
>              {
>                  strcpy(szMyStuffTarget, xdg_results[i]);
>                  break;
>              }
>
> -            /* Or the OS X folder (these are never localized) */
> +            /* Try to target the hardcoded / OS X 'XXX' folder. */
>              if (pszHome)
>              {
>                  strcpy(szMyStuffTarget, pszHome);
> @@ -4531,7 +4526,7 @@ static void _SHCreateSymbolicLinks(void)
>                      break;
>              }
>
> -            /* As a last resort point to the same location as 'My
> Documents' */
> +            /* Use the same target as '%USERPROFILE%\\My Documents'
> (fallback). */
>              strcpy(szMyStuffTarget, szPersonalTarget);
>              break;
>          }
> @@ -4540,7 +4535,7 @@ static void _SHCreateSymbolicLinks(void)
>          heap_free(pszMyStuff);
>      }
>
> -    /* Last but not least, the Desktop folder */
> +    /* Create a symbolic link for the 'Desktop' folder. */
>      if (pszHome)
>          strcpy(szDesktopTarget, pszHome);
>      else
> @@ -4552,6 +4547,7 @@ static void _SHCreateSymbolicLinks(void)
>          (_SHAppendToUnixPath(szDesktopTarget, DesktopW) &&
>          !stat(szDesktopTarget, &statFolder) &&
> S_ISDIR(statFolder.st_mode)))
>      {
> +        /* Get the '%USERPROFILE%\\Desktop' directory path. */
>          hr = SHGetFolderPathW(NULL, CSIDL_DESKTOPDIRECTORY|CSIDL_FLAG_CREATE,
> NULL,
>                                SHGFP_TYPE_DEFAULT, wszTempPath);
>          if (SUCCEEDED(hr) && (pszDesktop = wine_get_unix_file_name(wszTempPath)))
>
> @@ -4565,7 +4561,6 @@ static void _SHCreateSymbolicLinks(void)
>          }
>      }
>
> -    /* Free resources allocated by XDG_UserDirLookup() */
>      if (xdg_results)
>      {
>          for (i = 0; i < num; i++)
> --
> 2.18.0
>
>
This patchset is to primarily refactor: _SHCreateSymbolicLinks()
It is intended to make the existing code less opaque, more streamlined
and easier to work with. Although the _SHCreateSymbolicLinks() functionality
is quite simple, the existing implementation is hard to parse/
spaghetti code.

FYI I obviously still have patches ready to submit, which will fix:

* https://bugs.winehq.org/show_bug.cgi?id=41668
* https://bugs.winehq.org/show_bug.cgi?id=28216

that depend heavily on this code clean-up.
Plus an implementation of the Windows Vista(+) compatiblility junction
links (again which depends on this patchset).


All the best,
Robert
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20180718/0005d46a/attachment.html>


More information about the wine-devel mailing list