[PATCH 0/3] MR9145: taskschd: ITaskDefinition fixes

[Owen Rudge (＠orudge)]

Patch 1 fixes a bug that causes the `ITaskDefinition` to be freed as soon as the `IRegisteredTask` is created.

Patches 2 and 3 implement the ITaskDefinition Data property.

[PATCH 1/3] taskschd: Only release ITaskDefinition if creation of RegisteredTask fails.

[Owen Rudge]

From: Owen Rudge

---
 dlls/taskschd/regtask.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/dlls/taskschd/regtask.c b/dlls/taskschd/regtask.c
index 71164b268d6..4bc860c2d96 100644
--- a/dlls/taskschd/regtask.c
+++ b/dlls/taskschd/regtask.c
@@ -543,7 +543,10 @@ static HRESULT WINAPI regtasks_get_Item(IRegisteredTaskCollection *iface, VARIAN
     VariantClear(&converted_index);
     MIDL_user_free(task_names[0]);
     MIDL_user_free(task_names);
-    ITaskDefinition_Release(definition);
+
+    if (hr != S_OK)
+        ITaskDefinition_Release(definition);
+
     return hr;
 }

[PATCH 2/3] taskschd/tests: Add test for ITaskDefinition Data property.

[Owen Rudge]

From: Owen Rudge

---
 dlls/taskschd/tests/scheduler.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/dlls/taskschd/tests/scheduler.c b/dlls/taskschd/tests/scheduler.c
index 69dc681171a..cc7a6afdd76 100644
--- a/dlls/taskschd/tests/scheduler.c
+++ b/dlls/taskschd/tests/scheduler.c
@@ -1603,6 +1603,7 @@ static void test_TaskDefinition(void)
         "      <Command>"task1.exe"</Command>\n"
         "    </Exec>\n"
         "  </Actions>\n"
+        "  <Data>MyTestData</Data>\n"
         "</Task>\n";
     static WCHAR xml2[] =
         L"<Task>\n"
@@ -1843,6 +1844,25 @@ static void test_TaskDefinition(void)

IRegistrationInfo_Release(reginfo);

+    hr = ITaskDefinition_get_Data(taskdef, &bstr);
+    todo_wine ok(hr == S_OK, "get_Data error %#lx\n", hr);
+    if (!winetest_platform_is_wine) ok(!lstrcmpW(bstr, L"MyTestData"), "expected "MyTestData", got %s\n", wine_dbgstr_w(bstr));
+    if (!winetest_platform_is_wine) SysFreeString(bstr);
+
+    hr = ITaskDefinition_put_Data(taskdef, (BSTR) L"NewTest");
+    todo_wine ok(hr == S_OK, "put_Data error %#lx\n", hr);
+    bstr = (BSTR)0xdeadbeef;
+    hr = ITaskDefinition_get_Data(taskdef, &bstr);
+    if (!winetest_platform_is_wine) ok(!lstrcmpW(bstr, L"NewTest"), "expected "NewTest", got %s\n", wine_dbgstr_w(bstr));
+    if (!winetest_platform_is_wine) SysFreeString(bstr);
+
+    hr = ITaskDefinition_put_Data(taskdef, NULL);
+    todo_wine ok(hr == S_OK, "put_Data error %#lx\n", hr);
+    bstr = (BSTR)0xdeadbeef;
+    hr = ITaskDefinition_get_Data(taskdef, &bstr);
+    todo_wine ok(hr == S_OK, "get_Data error %#lx\n", hr);
+    todo_wine ok(!bstr, "expected NULL, got %s\n", wine_dbgstr_w(bstr));
+
     hr = ITaskDefinition_put_XmlText(taskdef, xml4);
     ok(hr == S_OK, "put_XmlText error %#lx\n", hr);
     hr = ITaskDefinition_get_RegistrationInfo(taskdef, &reginfo);

[PATCH 3/3] taskschd: Implement ITaskDefinition Data property.

[Owen Rudge]

From: Owen Rudge

---
 dlls/taskschd/task.c            | 56 ++++++++++++++++++++++++++++++---
 dlls/taskschd/tests/scheduler.c | 18 +++++------
 2 files changed, 61 insertions(+), 13 deletions(-)

diff --git a/dlls/taskschd/task.c b/dlls/taskschd/task.c
index a3378a99117..1fd94fe4df6 100644
--- a/dlls/taskschd/task.c
+++ b/dlls/taskschd/task.c
@@ -2373,6 +2373,7 @@ typedef struct
     ITriggerCollection *triggers;
     IPrincipal *principal;
     IActionCollection *actions;
+    BSTR data;
 } TaskDefinition;

static inline TaskDefinition *impl_from_ITaskDefinition(ITaskDefinition *iface)
@@ -2405,6 +2406,8 @@ static ULONG WINAPI TaskDefinition_Release(ITaskDefinition *iface)
             IPrincipal_Release(taskdef->principal);
         if (taskdef->actions)
             IActionCollection_Release(taskdef->actions);
+        if (taskdef->data)
+            SysFreeString(taskdef->data);

free(taskdef);
     }
@@ -2576,14 +2579,41 @@ static HRESULT WINAPI TaskDefinition_put_Settings(ITaskDefinition *iface, ITaskS

static HRESULT WINAPI TaskDefinition_get_Data(ITaskDefinition *iface, BSTR *data)
 {
-    FIXME("%p,%p: stub\n", iface, data);
-    return E_NOTIMPL;
+    TaskDefinition *taskdef = impl_from_ITaskDefinition(iface);
+
+    TRACE("%p,%p\n", iface, data);
+
+    if (data == NULL) return E_POINTER;
+
+    if (taskdef->data == NULL)
+        *data = NULL;
+    else
+    {
+        *data = SysAllocString(taskdef->data);
+        if (*data == NULL) return E_OUTOFMEMORY;
+    }
+
+    return S_OK;
 }

static HRESULT WINAPI TaskDefinition_put_Data(ITaskDefinition *iface, BSTR data)
 {
-    FIXME("%p,%p: stub\n", iface, data);
-    return E_NOTIMPL;
+    TaskDefinition *taskdef = impl_from_ITaskDefinition(iface);
+    BSTR copy = NULL;
+
+    TRACE("%p,%s\n", iface, debugstr_w(data));
+
+    if (data != NULL)
+    {
+        copy = SysAllocString(data);
+        if (copy == NULL) return E_OUTOFMEMORY;
+    }
+
+    if (taskdef->data != NULL)
+        SysFreeString(taskdef->data);
+
+    taskdef->data = copy;
+    return S_OK;
 }

static HRESULT WINAPI TaskDefinition_get_Principal(ITaskDefinition *iface, IPrincipal **principal)
@@ -3127,6 +3157,12 @@ static HRESULT WINAPI TaskDefinition_get_XmlText(ITaskDefinition *iface, BSTR *x
     hr = write_actions(stream, taskdef->actions);
     if (hr != S_OK) goto failed;

+    if (taskdef->data != NULL)
+    {
+        hr = write_text_value(stream, L"Data", taskdef->data);
+        if (hr != S_OK) goto failed;
+    }
+
     pop_indent();

write_element_end(stream, L"Task");
@@ -3766,6 +3802,13 @@ static HRESULT read_task(IXmlReader *reader, ITaskDefinition *taskdef)
                 hr = read_principals(reader, taskdef);
             else if (!lstrcmpW(name, L"Actions"))
                 hr = read_actions(reader, taskdef);
+            else if (!lstrcmpW(name, L"Data"))
+            {
+                WCHAR *value;
+                hr = read_text_value(reader, &value);
+                if (hr == S_OK)
+                    ITaskDefinition_put_Data(taskdef, value);
+            }
             else
                 FIXME("unhandled Task element %s\n", debugstr_w(name));

@@ -3894,6 +3937,11 @@ static HRESULT WINAPI TaskDefinition_put_XmlText(ITaskDefinition *iface, BSTR xm
             IActionCollection_Release(taskdef->actions);
             taskdef->actions = NULL;
         }
+        if (taskdef->data)
+        {
+            SysFreeString(taskdef->data);
+            taskdef->data = NULL;
+        }

hr = read_xml(reader, iface);
     }
diff --git a/dlls/taskschd/tests/scheduler.c b/dlls/taskschd/tests/scheduler.c
index cc7a6afdd76..615f8da8084 100644
--- a/dlls/taskschd/tests/scheduler.c
+++ b/dlls/taskschd/tests/scheduler.c
@@ -1845,23 +1845,23 @@ static void test_TaskDefinition(void)
     IRegistrationInfo_Release(reginfo);

hr = ITaskDefinition_get_Data(taskdef, &bstr);
-    todo_wine ok(hr == S_OK, "get_Data error %#lx\n", hr);
-    if (!winetest_platform_is_wine) ok(!lstrcmpW(bstr, L"MyTestData"), "expected "MyTestData", got %s\n", wine_dbgstr_w(bstr));
-    if (!winetest_platform_is_wine) SysFreeString(bstr);
+    ok(hr == S_OK, "get_Data error %#lx\n", hr);
+    ok(!lstrcmpW(bstr, L"MyTestData"), "expected "MyTestData", got %s\n", wine_dbgstr_w(bstr));
+    SysFreeString(bstr);

hr = ITaskDefinition_put_Data(taskdef, (BSTR) L"NewTest");
-    todo_wine ok(hr == S_OK, "put_Data error %#lx\n", hr);
+    ok(hr == S_OK, "put_Data error %#lx\n", hr);
     bstr = (BSTR)0xdeadbeef;
     hr = ITaskDefinition_get_Data(taskdef, &bstr);
-    if (!winetest_platform_is_wine) ok(!lstrcmpW(bstr, L"NewTest"), "expected "NewTest", got %s\n", wine_dbgstr_w(bstr));
-    if (!winetest_platform_is_wine) SysFreeString(bstr);
+    ok(!lstrcmpW(bstr, L"NewTest"), "expected "NewTest", got %s\n", wine_dbgstr_w(bstr));
+    SysFreeString(bstr);

hr = ITaskDefinition_put_Data(taskdef, NULL);
-    todo_wine ok(hr == S_OK, "put_Data error %#lx\n", hr);
+    ok(hr == S_OK, "put_Data error %#lx\n", hr);
     bstr = (BSTR)0xdeadbeef;
     hr = ITaskDefinition_get_Data(taskdef, &bstr);
-    todo_wine ok(hr == S_OK, "get_Data error %#lx\n", hr);
-    todo_wine ok(!bstr, "expected NULL, got %s\n", wine_dbgstr_w(bstr));
+    ok(hr == S_OK, "get_Data error %#lx\n", hr);
+    ok(!bstr, "expected NULL, got %s\n", wine_dbgstr_w(bstr));

hr = ITaskDefinition_put_XmlText(taskdef, xml4);
     ok(hr == S_OK, "put_XmlText error %#lx\n", hr);

[Dmitry Timoshkov (＠dmitry)]

Dmitry Timoshkov (@dmitry) commented about dlls/taskschd/task.c:
> {
> - FIXME("%p,%p: stub\n", iface, data);
> - return E_NOTIMPL;
> - TaskDefinition *taskdef = impl_from_ITaskDefinition(iface);
> -
> - TRACE("%p,%p\n", iface, data);
> -
> - if (data == NULL) return E_POINTER;
> -
> - if (taskdef->data == NULL)
> -
>    *data = NULL;
> - else
> - {
> -
>    *data = SysAllocString(taskdef->data);
> -
>    if (*data == NULL) return E_OUTOFMEMORY;
> - }

Please follow the existing code style:
if (!data) ...
if (!taskdef->data) ...
if (!*data) ...