Wine-Devel
By thread
wine-devel@list.winehq.org
By month
Messages by month
- ----- 2026 -----
- September
- August
- July
- June
- May
- April
- March
- February
- January
- ----- 2025 -----
- December
- November
- October
- September
- August
- July
- June
- May
- April
- March
- February
- January
- ----- 2024 -----
- December
- November
- October
- September
- August
- July
- June
- May
- April
- March
- February
- January
- ----- 2023 -----
- December
- November
- October
- September
- August
- July
- June
- May
- April
- March
- February
- January
- ----- 2022 -----
- December
- November
- October
- September
- August
- July
- June
- May
- April
- March
- February
- January
- ----- 2021 -----
- December
- November
- October
- September
- August
- July
- June
- May
- April
- March
- February
- January
- ----- 2020 -----
- December
- November
- October
- September
- August
- July
- June
- May
- April
- March
- February
- January
- ----- 2019 -----
- December
- November
- October
- September
- August
- July
- June
- May
- April
- March
- February
- January
- ----- 2018 -----
- December
- November
- October
- September
- August
- July
- June
- May
- April
- March
- February
- January
- ----- 2017 -----
- December
- November
- October
- September
- August
- July
- June
- May
- April
- March
- February
- January
- ----- 2016 -----
- December
- November
- October
- September
- August
- July
- June
- May
- April
- March
- February
- January
- ----- 2015 -----
- December
- November
- October
- September
- August
- July
- June
- May
- April
- March
- February
- January
- ----- 2014 -----
- December
- November
- October
- September
- August
- July
- June
- May
- April
- March
- February
- January
- ----- 2013 -----
- December
- November
- October
- September
- August
- July
- June
- May
- April
- March
- February
- January
- ----- 2012 -----
- December
- November
- October
- September
- August
- July
- June
- May
- April
- March
- February
- January
- ----- 2011 -----
- December
- November
- October
- September
- August
- July
- June
- May
- April
- March
- February
- January
- ----- 2010 -----
- December
- November
- October
- September
- August
- July
- June
- May
- April
- March
- February
- January
- ----- 2009 -----
- December
- November
- October
- September
- August
- July
- June
- May
- April
- March
- February
- January
- ----- 2008 -----
- December
- November
- October
- September
- August
- July
- June
- May
- April
- March
- February
- January
- ----- 2007 -----
- December
- November
- October
- September
- August
- July
- June
- May
- April
- March
- February
- January
- ----- 2006 -----
- December
- November
- October
- September
- August
- July
- June
- May
- April
- March
- February
- January
- ----- 2005 -----
- December
- November
- October
- September
- August
- July
- June
- May
- April
- March
- February
- January
- ----- 2004 -----
- December
- November
- October
- September
- August
- July
- June
- May
- April
- March
- February
- January
- ----- 2003 -----
- December
- November
- October
- September
- August
- July
- June
- May
- April
- March
- February
- January
- ----- 2002 -----
- December
- November
- October
- September
- August
- July
- June
- May
- April
- March
- February
- January
- ----- 2001 -----
- December
- November
- October
- September
- August
- July
- June
- May
- April
- March
- February
June 2018
- 68 participants
- 1149 messages
Re: [PATCH v2] shellpath.c: Fix creation of HOME directory symbolic links
by Huw Davies
On Fri, Jun 08, 2018 at 10:58:23AM +0100, Rob Walker wrote:
> Fixes: https://bugs.winehq.org/show_bug.cgi?id=41668
>
> The Shell Folders, that Wine symlinks to the user's HOME directory, are
> only re-created on each Wine boot if they: do not pre-exist or are broken
> symbolic links.
>
> Handling the DESKTOP Shell Folder is a special case as this is created twice
> (during a standard boot), early in the Wine boot process. This is handled by
> 2 separate processes, loading shell32.dll, in sequence. This makes it hard to determine
> if the DESKTOP folder was created before the Wine boot (either from a previous Wine boot
> or by the end user).
>
> The final (implemented) solution determines the exact start time of the Wine boot process
> (using the current system time and tick-count). If the DESKTOP directory was last written
> after this Wine boot time, we can assume that "we" (Wine) automatically created the
> directory. Only in this instance do we attempt to set a symlink to the DESKTOP directory
> (subdirectory of HOME).
There's still too much going on here.
Do you really need to refactor the code to make your change?
If not, then just send in the change to the current code.
If you need to refactor, then do the refactoring first
(I could imagine taking 3-4 patches to do the refactoring[1])
then make the change as a final patch in the series.
The file-time / boot-time thing seems hacky, I'm not exactly
sure what you're trying do to, but this doesn't sound right.
Hopefully that will become clearer as you tidy things up.
Huw.
[1] For example move the creation of My Pictures/My Videos/etc
first, then move My Documents and finally Desktop. These
final two are special cases in the current code, we'd need
to see that in any new code.
June 8, 2018
[PATCH v2] shellpath.c: Fix creation of HOME directory symbolic links
by Rob Walker
Fixes: https://bugs.winehq.org/show_bug.cgi?id=41668
The Shell Folders, that Wine symlinks to the user's HOME directory, are
only re-created on each Wine boot if they: do not pre-exist or are broken
symbolic links.
Handling the DESKTOP Shell Folder is a special case as this is created twice
(during a standard boot), early in the Wine boot process. This is handled by
2 separate processes, loading shell32.dll, in sequence. This makes it hard to determine
if the DESKTOP folder was created before the Wine boot (either from a previous Wine boot
or by the end user).
The final (implemented) solution determines the exact start time of the Wine boot process
(using the current system time and tick-count). If the DESKTOP directory was last written
after this Wine boot time, we can assume that "we" (Wine) automatically created the
directory. Only in this instance do we attempt to set a symlink to the DESKTOP directory
(subdirectory of HOME).
Test on Gentoo GNU/Linux.
Signed-off-by: Rob Walker <bob.mt.wya(a)gmail.com>
---
dlls/shell32/shellpath.c | 325 ++++++++++++++++++---------------------
1 file changed, 151 insertions(+), 174 deletions(-)
diff --git a/dlls/shell32/shellpath.c b/dlls/shell32/shellpath.c
index a551e93aa8..c87c73846b 100644
--- a/dlls/shell32/shellpath.c
+++ b/dlls/shell32/shellpath.c
@@ -58,6 +58,7 @@
WINE_DEFAULT_DEBUG_CHANNEL(shell);
static const BOOL is_win64 = sizeof(void *) > sizeof(int);
+static LONG register_shell_folders = 0;
/*
########## Combining and Constructing paths ##########
@@ -4098,6 +4099,15 @@ HRESULT WINAPI SHGetFolderPathAndSubDirW(
goto end;
}
+ /* Allow function create_homedir_symbolic_link create this directory
+ (or a HOME directory symlink) for us, later in the boot process. */
+ if (!InterlockedCompareExchange(®ister_shell_folders, -1, -1) && (folder == CSIDL_DESKTOPDIRECTORY))
+ {
+ TRACE("Faking successful creation of system directory %s (%#x)\n", debugstr_w(szBuildPath), folder);
+ hr = S_OK;
+ goto end;
+ }
+
/* create directory/directories */
ret = SHCreateDirectoryExW(hwndOwner, szBuildPath, NULL);
if (ret && ret != ERROR_ALREADY_EXISTS)
@@ -4107,7 +4117,7 @@ HRESULT WINAPI SHGetFolderPathAndSubDirW(
goto end;
}
- TRACE("Created missing system directory %s\n", debugstr_w(szBuildPath));
+ TRACE("Created missing system directory %s (%#x)\n", debugstr_w(szBuildPath), folder);
end:
TRACE("returning 0x%08x (final path is %s)\n", hr, debugstr_w(szBuildPath));
return hr;
@@ -4385,193 +4395,111 @@ static inline BOOL _SHAppendToUnixPath(char *szBasePath, LPCWSTR pwszSubPath) {
}
/******************************************************************************
- * _SHCreateSymbolicLinks [Internal]
- *
- * Sets up symbol links for various shell folders to point into the users home
- * directory. We do an educated guess about what the user would probably want:
- * - If there is a 'My Documents' directory in $HOME, the user probably wants
- * wine's 'My Documents' to point there. Furthermore, we imply that the user
- * is a Windows lover and has no problem with wine creating 'My Pictures',
- * 'My Music' and 'My Videos' subfolders under '$HOME/My Documents', if those
- * do not already exits. We put appropriate symbolic links in place for those,
- * too.
- * - If there is no 'My Documents' directory in $HOME, we let 'My Documents'
- * point directly to $HOME. We assume the user to be a unix hacker who does not
- * want wine to create anything anywhere besides the .wine directory. So, if
- * there already is a 'My Music' directory in $HOME, we symlink the 'My Music'
- * shell folder to it. But if not, then we check XDG_MUSIC_DIR - "well known"
- * directory, and try to link to that. If that fails, then we symlink to
- * $HOME directly. The same holds fo 'My Pictures' and 'My Videos'.
- * - The Desktop shell folder is symlinked to XDG_DESKTOP_DIR. If that does not
- * exist, then we try '$HOME/Desktop'. If that does not exist, then we leave
- * it alone.
- * ('My Music',... above in fact means LoadString(IDS_MYMUSIC))
+ * create_homedir_symbolic_link [Internal]
+ *
+ * Creates a symbolic link from the current Wineprefix to an appropriate
+ * HOME subdirectory (if one is found).
+ *
+ * Creates 'XXXX' directory in Wineprefix.
+ * Then create a 'My XXXX' symbolic link in Wineprefix:
+ * 1) If '$HOME/XXXX' (IDS directory) exists then target this.
+ * 2) If '$HOME/XXXX' (XDG_XXXX_DIR) exists then target this.
+ * 3) If '$HOME/XXXX' (MacOS XXXX media directory) exists then target this.
+ *
+ * PARAMS
+ * ids_dir [I] Windows Resource Identifier code for current Shell Folder.
+ * csidl_dir [I] Constant Special Item ID List identifier for current Shell Folder.
+ * xdg_dir [I] Full path of external Unix XDG directory corresponding to current Shell Folder.
+ * ws_osx_dir [I] Fallback directory name to use, corresponding to current Shell Folder (OSX specific).
+ *
*/
-static void _SHCreateSymbolicLinks(void)
-{
- UINT aidsMyStuff[] = { IDS_MYPICTURES, IDS_MYVIDEOS, IDS_MYMUSIC }, i;
- const WCHAR* MyOSXStuffW[] = { PicturesW, MoviesW, MusicW };
- int acsidlMyStuff[] = { CSIDL_MYPICTURES, CSIDL_MYVIDEO, CSIDL_MYMUSIC };
- static const char * const xdg_dirs[] = { "PICTURES", "VIDEOS", "MUSIC", "DOCUMENTS", "DESKTOP" };
- static const unsigned int num = ARRAY_SIZE(xdg_dirs);
- WCHAR wszTempPath[MAX_PATH];
- char szPersonalTarget[FILENAME_MAX], *pszPersonal;
- char szMyStuffTarget[FILENAME_MAX], *pszMyStuff;
- char szDesktopTarget[FILENAME_MAX], *pszDesktop;
- struct stat statFolder;
- const char *pszHome;
+void create_homedir_symbolic_link(UINT ids_dir,
+ int csidl_dir,
+ const char * xdg_dir,
+ const WCHAR * ws_osx_dir)
+{
+ static const char * env_homedir = NULL;
+ WCHAR ws_temp_path[MAX_PATH];
+ char home_target[FILENAME_MAX], * prefix_dir;
+ struct stat stat_folder, stat_home_folder;
HRESULT hr;
- char ** xdg_results;
- char * xdg_desktop_dir;
-
- /* Create all necessary profile sub-dirs up to 'My Documents' and get the unix path. */
- hr = SHGetFolderPathW(NULL, CSIDL_PERSONAL|CSIDL_FLAG_CREATE, NULL,
- SHGFP_TYPE_DEFAULT, wszTempPath);
- if (FAILED(hr)) return;
- pszPersonal = wine_get_unix_file_name(wszTempPath);
- if (!pszPersonal) return;
+ BOOL target_ok;
- hr = XDG_UserDirLookup(xdg_dirs, num, &xdg_results);
- if (FAILED(hr)) xdg_results = NULL;
-
- pszHome = getenv("HOME");
- if (pszHome && !stat(pszHome, &statFolder) && S_ISDIR(statFolder.st_mode))
+ hr = SHGetFolderPathW(NULL, csidl_dir, NULL,
+ SHGFP_TYPE_DEFAULT, ws_temp_path);
+ if (SUCCEEDED(hr))
{
- while (1)
- {
- /* Check if there's already a Wine-specific 'My Documents' folder */
- strcpy(szPersonalTarget, pszHome);
- if (_SHAppendToUnixPath(szPersonalTarget, MAKEINTRESOURCEW(IDS_PERSONAL)) &&
- !stat(szPersonalTarget, &statFolder) && S_ISDIR(statFolder.st_mode))
- {
- /* '$HOME/My Documents' exists. Create 'My Pictures',
- * 'My Videos' and 'My Music' subfolders or fail silently if
- * they already exist.
- */
- for (i = 0; i < ARRAY_SIZE(aidsMyStuff); i++)
- {
- strcpy(szMyStuffTarget, szPersonalTarget);
- if (_SHAppendToUnixPath(szMyStuffTarget, MAKEINTRESOURCEW(aidsMyStuff[i])))
- mkdir(szMyStuffTarget, 0777);
- }
- 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))
- {
- strcpy(szPersonalTarget, xdg_results[num-2]);
- break;
- }
-
- /* Or the hardcoded / OS X Documents folder */
- strcpy(szPersonalTarget, pszHome);
- if (_SHAppendToUnixPath(szPersonalTarget, DocumentsW) &&
- !stat(szPersonalTarget, &statFolder) &&
- S_ISDIR(statFolder.st_mode))
- break;
-
- /* As a last resort point to $HOME. */
- strcpy(szPersonalTarget, pszHome);
- break;
- }
-
- /* Replace 'My Documents' directory with a symlink or fail silently if not empty. */
- remove(pszPersonal);
- symlink(szPersonalTarget, pszPersonal);
+ TRACE("%s directory already exists\n",
+ debugstr_w(ws_temp_path));
+ return;
+ }
+ else if (hr == HRESULT_FROM_WIN32(ERROR_FILE_NOT_FOUND))
+ {
+ hr = SHGetFolderPathW(NULL, csidl_dir|CSIDL_FLAG_CREATE, NULL,
+ SHGFP_TYPE_DEFAULT, ws_temp_path);
+ if (FAILED(hr)) return;
}
else
{
- /* '$HOME' doesn't exist. Create 'My Pictures', 'My Videos' and 'My Music' subdirs
- * in '%USERPROFILE%\\My Documents' or fail silently if they already exist. */
- pszHome = NULL;
- strcpy(szPersonalTarget, pszPersonal);
- for (i = 0; i < ARRAY_SIZE(aidsMyStuff); i++) {
- strcpy(szMyStuffTarget, szPersonalTarget);
- if (_SHAppendToUnixPath(szMyStuffTarget, MAKEINTRESOURCEW(aidsMyStuff[i])))
- mkdir(szMyStuffTarget, 0777);
- }
+ ERR("Failed to get Wineprefix path corresponding to %d CSIDL\n",
+ csidl_dir);
+ return;
}
- /* Create symbolic links for 'My Pictures', 'My Videos' and 'My Music'. */
- for (i=0; i < ARRAY_SIZE(aidsMyStuff); i++)
+ prefix_dir = wine_get_unix_file_name(ws_temp_path);
+ if (!prefix_dir)
{
- /* Create the current 'My Whatever' folder and get its unix 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 */
- strcpy(szMyStuffTarget, szPersonalTarget);
- if (_SHAppendToUnixPath(szMyStuffTarget, MAKEINTRESOURCEW(aidsMyStuff[i])) &&
- !stat(szMyStuffTarget, &statFolder) && S_ISDIR(statFolder.st_mode))
- break;
-
- /* Try 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) */
- if (pszHome)
- {
- strcpy(szMyStuffTarget, pszHome);
- if (_SHAppendToUnixPath(szMyStuffTarget, MyOSXStuffW[i]) &&
- !stat(szMyStuffTarget, &statFolder) &&
- S_ISDIR(statFolder.st_mode))
- break;
- }
+ ERR("Failed to get Unix Wineprefix directory for: %s\n",
+ debugstr_w(ws_temp_path));
+ return;
+ }
- /* As a last resort point to the same location as 'My Documents' */
- strcpy(szMyStuffTarget, szPersonalTarget);
- break;
- }
- remove(pszMyStuff);
- symlink(szMyStuffTarget, pszMyStuff);
- heap_free(pszMyStuff);
+ if (!env_homedir) env_homedir = getenv("HOME");
+ if (!(env_homedir
+ && (stat(env_homedir, &stat_home_folder) != 1)
+ && S_ISDIR(stat_home_folder.st_mode)))
+ {
+ if (prefix_dir) heap_free(prefix_dir);
+ return;
}
- /* Last but not least, the Desktop folder */
- if (pszHome)
- strcpy(szDesktopTarget, pszHome);
+ /* Check for:
+ * '$HOME/XXXX' (IDS directory)
+ * or '$HOME/XXXX' (XDG directory)
+ * or '$HOME/XXXX' (MacOS directory)
+ */
+ target_ok = FALSE;
+ strcpy(home_target, env_homedir);
+ if (_SHAppendToUnixPath(home_target, MAKEINTRESOURCEW(ids_dir))
+ && (stat(home_target, &stat_folder) != -1)
+ && S_ISDIR(stat_folder.st_mode))
+ {
+ target_ok = TRUE;
+ }
+ else if (xdg_dir)
+ {
+ strcpy(home_target, xdg_dir);
+ /* Only link to the XDG directory, if it does not point directly
+ * to the user's HOME directory (XDG specification fallback path). */
+ target_ok = TRUE;
+ }
else
- strcpy(szDesktopTarget, pszPersonal);
- heap_free(pszPersonal);
-
- xdg_desktop_dir = xdg_results ? xdg_results[num - 1] : NULL;
- if (xdg_desktop_dir ||
- (_SHAppendToUnixPath(szDesktopTarget, DesktopW) &&
- !stat(szDesktopTarget, &statFolder) && S_ISDIR(statFolder.st_mode)))
{
- hr = SHGetFolderPathW(NULL, CSIDL_DESKTOPDIRECTORY|CSIDL_FLAG_CREATE, NULL,
- SHGFP_TYPE_DEFAULT, wszTempPath);
- if (SUCCEEDED(hr) && (pszDesktop = wine_get_unix_file_name(wszTempPath)))
- {
- remove(pszDesktop);
- if (xdg_desktop_dir)
- symlink(xdg_desktop_dir, pszDesktop);
- else
- symlink(szDesktopTarget, pszDesktop);
- heap_free(pszDesktop);
- }
+ strcpy(home_target, env_homedir);
+ target_ok = _SHAppendToUnixPath(home_target, ws_osx_dir)
+ && (stat(home_target, &stat_folder) != -1)
+ && S_ISDIR(stat_folder.st_mode);
}
- /* Free resources allocated by XDG_UserDirLookup() */
- if (xdg_results)
+ if (target_ok)
{
- for (i = 0; i < num; i++)
- heap_free(xdg_results[i]);
- heap_free(xdg_results);
+ TRACE("Delete path: %s\n", debugstr_a(prefix_dir));
+ remove(prefix_dir);
+ TRACE("Symlink: %s -> %s\n", debugstr_a(prefix_dir), debugstr_a(home_target));
+ symlink(home_target, prefix_dir);
}
+
+ if (prefix_dir) heap_free(prefix_dir);
}
/******************************************************************************
@@ -6126,15 +6054,61 @@ static void register_system_knownfolders(void)
}
}
+/******************************************************************************
+ * create_homedir_symbolic_links [Internal]
+ *
+ * Parse WINESYMLINK env variable, for each XDG directory argument. To test if symlinking is
+ * enabled for that XDG directory / Wine Profile Folder.
+ * Then calls the function create_homedir_symbolic_link to potentially symlink from a Shell Folder,
+ * in the current Wineprefix, to a subdirectory of the current user's HOME directory.
+ *
+ * PARAMS
+ * xdg_dirnames [I] Pointer to an array of Unix XDG directory names
+ * (without "XDG_" prefix and "_DIR" suffix).
+ * xdg_dir_count [I] Item count of array (above).
+ *
+ */
+static void create_homedir_symbolic_links(const char * const xdg_dirnames[], const UINT xdg_dir_count)
+{
+ char ** xdg_dirs_array;
+ char * xdg_dir;
+ HRESULT hr;
+ UINT i;
+
+ if (!xdg_dirnames) return;
+
+ hr = XDG_UserDirLookup(xdg_dirnames, xdg_dir_count, &xdg_dirs_array);
+ if (FAILED(hr)) xdg_dirs_array = NULL;
+
+ for (i = 0; i < xdg_dir_count; ++i)
+ {
+ xdg_dir = xdg_dirs_array ? xdg_dirs_array[i] : NULL;
+ if (!strcmp(xdg_dirnames[i],"DOCUMENTS"))
+ create_homedir_symbolic_link(IDS_PERSONAL, CSIDL_PERSONAL, xdg_dir, DocumentsW);
+ else if (!strcmp(xdg_dirnames[i],"PICTURES"))
+ create_homedir_symbolic_link(IDS_MYPICTURES, CSIDL_MYPICTURES, xdg_dir, PicturesW);
+ else if (!strcmp(xdg_dirnames[i],"VIDEOS"))
+ create_homedir_symbolic_link(IDS_MYVIDEOS, CSIDL_MYVIDEO, xdg_dir, MoviesW);
+ else if (!strcmp(xdg_dirnames[i],"MUSIC"))
+ create_homedir_symbolic_link(IDS_MYMUSIC, CSIDL_MYMUSIC, xdg_dir, MusicW);
+ else if (!strcmp(xdg_dirnames[i],"DESKTOP"))
+ create_homedir_symbolic_link(IDS_DESKTOPDIRECTORY, CSIDL_DESKTOPDIRECTORY, xdg_dir, DesktopW);
+ else
+ ERR("XDG directory name specifier invalid: %s\n", debugstr_a(xdg_dirnames[i]));
+ if (xdg_dir) heap_free(xdg_dirs_array[i]);
+ }
+ if (xdg_dirs_array) heap_free(xdg_dirs_array);
+}
+
HRESULT SHELL_RegisterShellFolders(void)
{
+ static const char * const xdg_dirnames[] = { "DOCUMENTS", "PICTURES", "VIDEOS", "MUSIC", "DESKTOP" };
+ const UINT xdg_dir_count = 5;
HRESULT hr;
- /* Set up '$HOME' targeted symlinks for 'My Documents', 'My Pictures',
- * 'My Videos', 'My Music' and 'Desktop' in advance, so that the
- * _SHRegister*ShellFolders() functions will find everything nice and clean
- * and thus will not attempt to create them in the profile directory. */
- _SHCreateSymbolicLinks();
+ /* Early setup of symlinks from specific User Shell Folders, in
+ * current Wineprefix, to subdirecties of the user's HOME directory. */
+ create_homedir_symbolic_links(xdg_dirnames, xdg_dir_count);
hr = _SHRegisterUserShellFolders(TRUE);
if (SUCCEEDED(hr))
@@ -6147,5 +6121,8 @@ HRESULT SHELL_RegisterShellFolders(void)
hr = set_folder_attributes();
if (SUCCEEDED(hr))
register_system_knownfolders();
+
+ (void) InterlockedExchange(®ister_shell_folders, 1);
+
return hr;
}
--
2.17.1
June 8, 2018
Re: [PATCH 4/4] wsdapi: Add initial support for reading messages; identify Probe message.
by Owen Rudge
> It's mainly just for consistency. I don't feel particularly strongly
> about it in these cases, however in previous patch-sets I let similar
> things go, only for it later to become apparent that I shouldn't have.
Although these functions are not used in my upcoming patches, I suppose it's feasible that one day they could be used by callers that could make use of the result, so I'll look at amending that.
> It certainly can't be left like this at the end of the patch-set. It
> might be ok (since this isn't exactly a core dll) to include the next
> patch so that the implementation doesn't regress as far as the tests
> are concerned.
OK, I can include the patch that will fix the TODO in the patchset, which I think would likely be the simplest way of resolving this.
Thanks,
Owen
June 8, 2018
Re: [PATCH 4/4] wsdapi: Add initial support for reading messages; identify Probe message.
by Huw Davies
On Thu, Jun 07, 2018 at 09:15:02PM +0100, Owen Rudge wrote:
> On 07/06/2018 08:56, Huw Davies wrote:
>
> >> +static BOOL move_to_element(WS_XML_READER *reader, const char
> *element_name, WS_XML_STRING *uri)
> >
> > return HRESULT
> >
> >> +static BOOL ws_element_to_wsdxml_element(WS_XML_READER *reader,
> IWSDXMLContext *context, WSDXML_ELEMENT *parent_element)
> >
> > and here.
> >
> >> int read_message(const char *xml, int xml_length, WSD_SOAP_MESSAGE
> **out_msg)
> >
> > This is also looking like a good contender to return HRESULT and
> > take an [out] int *type parameter.
>
> As per the previous patch, the HRESULT is not ultimately used, and I can't
> see that there's a great benefit in passing it along. I do understand it for
> the other functions that ultimately return a value to the caller, but these
> functions are all called by the listener thread, and a simple identification
> of the message type is all we require.
>
> If you think it would be beneficial, I can modify these functions to return
> HRESULT, but it would be good to understand the benefits of doing so.
It's mainly just for consistency. I don't feel particularly strongly
about it in these cases, however in previous patch-sets I let similar
things go, only for it later to become apparent that I shouldn't have.
One way to look at it is that conceptually the function has no idea
that the caller cannot use the failure status, so it should return
it regardless.
> >> - ok(any != NULL, "%s: any == NULL\n", debug_prefix);
> >> + todo_wine ok(any != NULL, "%s: any == NULL\n", debug_prefix);
> >
> > You'll need to have a really good reason for doing this. Can you
> > re-order things so you don't break the tests?
>
> This will be fixed in the next patchset; I didn't want to include any more
> code in what's already quite a large patch. There's probably an extra 40 or
> so lines of code needed to avoid this breakage. There's not really a way I
> can re-order this that I can think of.
>
> If preferred though, I can submit the patch that will fix this as part of
> this patchset, so the net result once the set is committed involves no extra
> todos.
It certainly can't be left like this at the end of the patch-set. It
might be ok (since this isn't exactly a core dll) to include the next
patch so that the implementation doesn't regress as far as the tests
are concerned.
Huw.
June 8, 2018
[PATCH 5/5] mstask/tests: Add more tests for IEnumWorkItems interface.
by Dmitry Timoshkov
Signed-off-by: Dmitry Timoshkov <dmitry(a)baikal.ru>
---
dlls/mstask/tests/task_scheduler.c | 75 +++++++++++++++++++++++++++++++++++---
1 file changed, 70 insertions(+), 5 deletions(-)
diff --git a/dlls/mstask/tests/task_scheduler.c b/dlls/mstask/tests/task_scheduler.c
index d4766382ec..f61022575d 100644
--- a/dlls/mstask/tests/task_scheduler.c
+++ b/dlls/mstask/tests/task_scheduler.c
@@ -212,23 +212,88 @@ done:
static void test_Enum(void)
{
+ static const WCHAR Task1[] = { 'w','i','n','e','t','a','s','k','1',0 };
ITaskScheduler *scheduler;
+ ITask *task;
IEnumWorkItems *tasks;
+ WCHAR **names;
+ ULONG fetched;
HRESULT hr;
hr = CoCreateInstance(&CLSID_CTaskScheduler, NULL, CLSCTX_INPROC_SERVER,
&IID_ITaskScheduler, (void **)&scheduler);
- ok(hr == S_OK, "got 0x%08x\n", hr);
+ ok(hr == S_OK, "got %#x\n", hr);
+
+ /* cleanup after previous runs */
+ ITaskScheduler_Delete(scheduler, Task1);
+
+ hr = ITaskScheduler_NewWorkItem(scheduler, Task1, &CLSID_CTask, &IID_ITask, (IUnknown **)&task);
+ ok(hr == S_OK, "got %#x\n", hr);
+ hr = ITaskScheduler_AddWorkItem(scheduler, Task1, (IScheduledWorkItem *)task);
+ ok(hr == S_OK, "got %#x\n", hr);
+
+ ITask_Release(task);
-if (0) { /* crashes on win2k */
hr = ITaskScheduler_Enum(scheduler, NULL);
- ok(hr == E_INVALIDARG, "got 0x%08x\n", hr);
-}
+ ok(hr == E_INVALIDARG, "got %#x\n", hr);
hr = ITaskScheduler_Enum(scheduler, &tasks);
- ok(hr == S_OK, "got 0x%08x\n", hr);
+ ok(hr == S_OK, "got %#x\n", hr);
+
+ names = (void *)0xdeadbeef;
+ fetched = 0xdeadbeef;
+ hr = IEnumWorkItems_Next(tasks, 0, &names, &fetched);
+ ok(hr == E_INVALIDARG, "got %#x\n", hr);
+ ok(names == (void *)0xdeadbeef, "got %p\n", names);
+ ok(fetched == 0xdeadbeef, "got %#x\n", fetched);
+
+ hr = IEnumWorkItems_Next(tasks, 1, NULL, NULL);
+ ok(hr == E_INVALIDARG, "got %#x\n", hr);
+
+ names = NULL;
+ hr = IEnumWorkItems_Next(tasks, 1, &names, NULL);
+ ok(hr == S_OK, "got %#x\n", hr);
+ ok(names != NULL, "got NULL\n");
+ ok(names[0] != NULL, "got NULL\n");
+ CoTaskMemFree(names[0]);
+ CoTaskMemFree(names);
+
+ names = (void *)0xdeadbeef;
+ hr = IEnumWorkItems_Next(tasks, 2, &names, NULL);
+ ok(hr == E_INVALIDARG, "got %#x\n", hr);
+ ok(names == (void *)0xdeadbeef, "got %p\n", names);
+
+ hr = IEnumWorkItems_Reset(tasks);
+ ok(hr == S_OK, "got %#x\n", hr);
+
+ names = NULL;
+ fetched = 0xdeadbeef;
+ hr = IEnumWorkItems_Next(tasks, 1, &names, &fetched);
+ ok(hr == S_OK, "got %#x\n", hr);
+ ok(names != NULL, "got NULL\n");
+ ok(names[0] != NULL, "got NULL\n");
+ ok(fetched == 1, "got %u\n", fetched);
+ CoTaskMemFree(names[0]);
+ CoTaskMemFree(names);
+
+ while (IEnumWorkItems_Skip(tasks, 1) == S_OK)
+ /* do nothing*/;
+
+ hr = IEnumWorkItems_Skip(tasks, 1);
+ ok(hr == S_FALSE, "got %#x\n", hr);
+
+ names = (void *)0xdeadbeef;
+ fetched = 0xdeadbeef;
+ hr = IEnumWorkItems_Next(tasks, 1, &names, &fetched);
+ ok(hr == S_FALSE, "got %#x\n", hr);
+ ok(names == NULL, "got %p\n", names);
+ ok(fetched == 0, "got %u\n", fetched);
+
IEnumWorkItems_Release(tasks);
+ hr = ITaskScheduler_Delete(scheduler, Task1);
+ ok(hr == S_OK, "got %#x\n", hr);
+
ITaskScheduler_Release(scheduler);
}
--
2.16.3
June 8, 2018
[PATCH 4/5] mstask: Implement IEnumWorkItems::Skip().
by Dmitry Timoshkov
Signed-off-by: Dmitry Timoshkov <dmitry(a)baikal.ru>
---
dlls/mstask/task_scheduler.c | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/dlls/mstask/task_scheduler.c b/dlls/mstask/task_scheduler.c
index ccb6d6c078..262dd0c49f 100644
--- a/dlls/mstask/task_scheduler.c
+++ b/dlls/mstask/task_scheduler.c
@@ -206,9 +206,17 @@ static HRESULT WINAPI EnumWorkItems_Next(IEnumWorkItems *iface, ULONG count, LPW
static HRESULT WINAPI EnumWorkItems_Skip(IEnumWorkItems *iface, ULONG count)
{
- EnumWorkItemsImpl *This = impl_from_IEnumWorkItems(iface);
- FIXME("(%p)->(%u): stub\n", This, count);
- return E_NOTIMPL;
+ LPWSTR *names;
+ ULONG fetched;
+ HRESULT hr;
+
+ TRACE("(%p)->(%u)\n", iface, count);
+
+ hr = EnumWorkItems_Next(iface, count, &names, &fetched);
+ if (SUCCEEDED(hr))
+ free_list(names, fetched);
+
+ return hr;
}
static HRESULT WINAPI EnumWorkItems_Reset(IEnumWorkItems *iface)
--
2.16.3
June 8, 2018
[PATCH 3/5] mstask: Implement IEnumWorkItems::Reset().
by Dmitry Timoshkov
Signed-off-by: Dmitry Timoshkov <dmitry(a)baikal.ru>
---
dlls/mstask/task_scheduler.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/dlls/mstask/task_scheduler.c b/dlls/mstask/task_scheduler.c
index 8db6e28b5e..ccb6d6c078 100644
--- a/dlls/mstask/task_scheduler.c
+++ b/dlls/mstask/task_scheduler.c
@@ -214,8 +214,16 @@ static HRESULT WINAPI EnumWorkItems_Skip(IEnumWorkItems *iface, ULONG count)
static HRESULT WINAPI EnumWorkItems_Reset(IEnumWorkItems *iface)
{
EnumWorkItemsImpl *This = impl_from_IEnumWorkItems(iface);
- FIXME("(%p): stub\n", This);
- return E_NOTIMPL;
+
+ TRACE("(%p)\n", This);
+
+ if (This->handle != INVALID_HANDLE_VALUE)
+ {
+ FindClose(This->handle);
+ This->handle = INVALID_HANDLE_VALUE;
+ }
+
+ return S_OK;
}
static HRESULT WINAPI EnumWorkItems_Clone(IEnumWorkItems *iface, IEnumWorkItems **cloned)
--
2.16.3
June 8, 2018
[PATCH 2/5] mstask: Implement IEnumWorkItems::Next().
by Dmitry Timoshkov
Signed-off-by: Dmitry Timoshkov <dmitry(a)baikal.ru>
---
dlls/mstask/task_scheduler.c | 98 +++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 96 insertions(+), 2 deletions(-)
diff --git a/dlls/mstask/task_scheduler.c b/dlls/mstask/task_scheduler.c
index 416a3218d0..8db6e28b5e 100644
--- a/dlls/mstask/task_scheduler.c
+++ b/dlls/mstask/task_scheduler.c
@@ -43,6 +43,7 @@ typedef struct
{
IEnumWorkItems IEnumWorkItems_iface;
LONG ref;
+ HANDLE handle;
} EnumWorkItemsImpl;
static inline TaskSchedulerImpl *impl_from_ITaskScheduler(ITaskScheduler *iface)
@@ -97,6 +98,8 @@ static ULONG WINAPI EnumWorkItems_Release(IEnumWorkItems *iface)
if (ref == 0)
{
+ if (This->handle != INVALID_HANDLE_VALUE)
+ FindClose(This->handle);
heap_free(This);
InterlockedDecrement(&dll_ref);
}
@@ -104,11 +107,101 @@ static ULONG WINAPI EnumWorkItems_Release(IEnumWorkItems *iface)
return ref;
}
+static void free_list(LPWSTR *list, LONG count)
+{
+ LONG i;
+
+ for (i = 0; i < count; i++)
+ CoTaskMemFree(list[i]);
+
+ CoTaskMemFree(list);
+}
+
+static inline BOOL is_file(const WIN32_FIND_DATAW *data)
+{
+ return !(data->dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY);
+}
+
static HRESULT WINAPI EnumWorkItems_Next(IEnumWorkItems *iface, ULONG count, LPWSTR **names, ULONG *fetched)
{
+ static const WCHAR tasksW[] = { '\\','T','a','s','k','s','\\','*',0 };
EnumWorkItemsImpl *This = impl_from_IEnumWorkItems(iface);
- FIXME("(%p)->(%u %p %p): stub\n", This, count, names, fetched);
- return E_NOTIMPL;
+ WCHAR path[MAX_PATH];
+ WIN32_FIND_DATAW data;
+ ULONG enumerated, dummy;
+ LPWSTR *list;
+ HRESULT hr = S_FALSE;
+
+ TRACE("(%p)->(%u %p %p)\n", This, count, names, fetched);
+
+ if (!count || !names || (!fetched && count > 1)) return E_INVALIDARG;
+
+ if (!fetched) fetched = &dummy;
+
+ *names = NULL;
+ *fetched = 0;
+ enumerated = 0;
+ list = NULL;
+
+ if (This->handle == INVALID_HANDLE_VALUE)
+ {
+ GetWindowsDirectoryW(path, MAX_PATH);
+ lstrcatW(path, tasksW);
+ This->handle = FindFirstFileW(path, &data);
+ if (This->handle == INVALID_HANDLE_VALUE)
+ return S_FALSE;
+ }
+ else
+ {
+ if (!FindNextFileW(This->handle, &data))
+ return S_FALSE;
+ }
+
+ do
+ {
+ if (is_file(&data))
+ {
+ LPWSTR *new_list;
+
+ if (!list)
+ new_list = CoTaskMemAlloc((enumerated + 1) * sizeof(list[0]));
+ else
+ new_list = CoTaskMemRealloc(list, (enumerated + 1) * sizeof(list[0]));
+ if (!new_list)
+ {
+ hr = E_OUTOFMEMORY;
+ break;
+ }
+
+ list = new_list;
+
+ list[enumerated] = CoTaskMemAlloc((lstrlenW(data.cFileName) + 1) * sizeof(WCHAR));
+ if (!list[enumerated])
+ {
+ hr = E_OUTOFMEMORY;
+ break;
+ }
+
+ lstrcpyW(list[enumerated], data.cFileName);
+ enumerated++;
+
+ if (enumerated >= count)
+ {
+ hr = S_OK;
+ break;
+ }
+ }
+ } while (FindNextFileW(This->handle, &data));
+
+ if (FAILED(hr))
+ free_list(list, enumerated);
+ else
+ {
+ *fetched = enumerated;
+ *names = list;
+ }
+
+ return hr;
}
static HRESULT WINAPI EnumWorkItems_Skip(IEnumWorkItems *iface, ULONG count)
@@ -154,6 +247,7 @@ static HRESULT create_task_enum(IEnumWorkItems **ret)
tasks->IEnumWorkItems_iface.lpVtbl = &EnumWorkItemsVtbl;
tasks->ref = 1;
+ tasks->handle = INVALID_HANDLE_VALUE;
*ret = &tasks->IEnumWorkItems_iface;
InterlockedIncrement(&dll_ref);
--
2.16.3
June 8, 2018
[PATCH 1/5] mstask: Implement ITask::GetExitCode().
by Dmitry Timoshkov
Signed-off-by: Dmitry Timoshkov <dmitry(a)baikal.ru>
---
dlls/mstask/task.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/dlls/mstask/task.c b/dlls/mstask/task.c
index 266029a58e..7499826937 100644
--- a/dlls/mstask/task.c
+++ b/dlls/mstask/task.c
@@ -685,8 +685,14 @@ static HRESULT WINAPI MSTASK_ITask_GetExitCode(ITask *iface, DWORD *exit_code)
TRACE("(%p, %p)\n", iface, exit_code);
+ if (This->status == SCHED_S_TASK_NOT_SCHEDULED)
+ {
+ *exit_code = 0;
+ return SCHED_S_TASK_HAS_NOT_RUN;
+ }
+
*exit_code = This->exit_code;
- return SCHED_S_TASK_HAS_NOT_RUN; /* FIXME */
+ return S_OK;
}
static HRESULT WINAPI MSTASK_ITask_SetComment(ITask *iface, LPCWSTR comment)
--
2.16.3
June 8, 2018
[PATCH 3/3] schedsvc: Add support for running missed tasks at the service start.
by Dmitry Timoshkov
Signed-off-by: Dmitry Timoshkov <dmitry(a)baikal.ru>
---
dlls/schedsvc/atsvc.c | 24 ++++++++++++++++++++++++
dlls/schedsvc/schedsvc_private.h | 1 +
dlls/schedsvc/svc_main.c | 1 +
3 files changed, 26 insertions(+)
diff --git a/dlls/schedsvc/atsvc.c b/dlls/schedsvc/atsvc.c
index b5b8317ded..f6ea0f0e23 100644
--- a/dlls/schedsvc/atsvc.c
+++ b/dlls/schedsvc/atsvc.c
@@ -1039,6 +1039,30 @@ void check_task_time(void)
LeaveCriticalSection(&at_job_list_section);
}
+void check_missed_task_time(void)
+{
+ FILETIME current_ft, last_ft;
+ struct job_t *job;
+
+ GetSystemTimeAsFileTime(¤t_ft);
+ FileTimeToLocalFileTime(¤t_ft, ¤t_ft);
+
+ EnterCriticalSection(&at_job_list_section);
+
+ LIST_FOR_EACH_ENTRY(job, &at_job_list, struct job_t, entry)
+ {
+ if (SystemTimeToFileTime(&job->data.last_runtime, &last_ft))
+ {
+ if (job_runs_at(job, &last_ft, ¤t_ft))
+ {
+ run_job(job);
+ }
+ }
+ }
+
+ LeaveCriticalSection(&at_job_list_section);
+}
+
void remove_job(const WCHAR *name)
{
struct job_t *job;
diff --git a/dlls/schedsvc/schedsvc_private.h b/dlls/schedsvc/schedsvc_private.h
index 10892d55ca..117c6f51ed 100644
--- a/dlls/schedsvc/schedsvc_private.h
+++ b/dlls/schedsvc/schedsvc_private.h
@@ -31,6 +31,7 @@ void update_process_status(DWORD pid) DECLSPEC_HIDDEN;
BOOL get_next_runtime(LARGE_INTEGER *rt) DECLSPEC_HIDDEN;
void check_task_time(void) DECLSPEC_HIDDEN;
void load_at_tasks(void) DECLSPEC_HIDDEN;
+void check_missed_task_time(void) DECLSPEC_HIDDEN;
static inline WCHAR *heap_strdupW(const WCHAR *src)
{
diff --git a/dlls/schedsvc/svc_main.c b/dlls/schedsvc/svc_main.c
index 2bba674b07..8e23a049db 100644
--- a/dlls/schedsvc/svc_main.c
+++ b/dlls/schedsvc/svc_main.c
@@ -54,6 +54,7 @@ static DWORD WINAPI tasks_monitor_thread(void *arg)
TRACE("Starting...\n");
load_at_tasks();
+ check_missed_task_time();
htimer = CreateWaitableTimerW(NULL, FALSE, NULL);
if (htimer == NULL)
--
2.16.3
June 8, 2018