[PATCH v3 0/1] MR1660: mstask: Don't assume a ONCE trigger is on the current day.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52994 This fixes a regression. -- v3: mstask: Don't assume a ONCE trigger is on the current day. https://gitlab.winehq.org/wine/wine/-/merge_requests/1660
From: Sven Baars <sbaars(a)codeweavers.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52994 --- dlls/mstask/task.c | 3 +++ dlls/mstask/tests/task_trigger.c | 16 ++++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/dlls/mstask/task.c b/dlls/mstask/task.c index d2a03aa8b56..3f3637f13a1 100644 --- a/dlls/mstask/task.c +++ b/dlls/mstask/task.c @@ -516,6 +516,9 @@ static HRESULT WINAPI MSTASK_ITask_GetNextRunTime(ITask *iface, SYSTEMTIME *rt) case TASK_TIME_TRIGGER_ONCE: st = current_st; + st.wYear = This->trigger[i].wBeginYear; + st.wMonth = This->trigger[i].wBeginMonth; + st.wDay = This->trigger[i].wBeginDay; st.wHour = This->trigger[i].wStartHour; st.wMinute = This->trigger[i].wStartMinute; st.wSecond = 0; diff --git a/dlls/mstask/tests/task_trigger.c b/dlls/mstask/tests/task_trigger.c index 24b873770d2..236540cf0ee 100644 --- a/dlls/mstask/tests/task_trigger.c +++ b/dlls/mstask/tests/task_trigger.c @@ -525,6 +525,22 @@ static void test_GetNextRunTime(void) hr = ITaskTrigger_SetTrigger(trigger, &data); ok(hr == S_OK, "got %#lx\n", hr); + memset(&st, 0xff, sizeof(st)); + hr = ITask_GetNextRunTime(task, &st); + ok(hr == S_OK, "got %#lx\n", hr); + ok(!memcmp(&st, &cmp, sizeof(st)), "got %u/%u/%u wday %u %u:%02u:%02u\n", + st.wDay, st.wMonth, st.wYear, st.wDayOfWeek, + st.wHour, st.wMinute, st.wSecond); + + hr = ITaskTrigger_GetTrigger(trigger, &data); + ok(hr == S_OK, "got %#lx\n", hr); + data.rgFlags &= ~TASK_TRIGGER_FLAG_DISABLED; + data.TriggerType = TASK_TIME_TRIGGER_ONCE; + /* add 1 day to test triggers in the far future */ + trigger_add_ms(&data, 24 * 60 * 60 * 1000, &cmp); + hr = ITaskTrigger_SetTrigger(trigger, &data); + ok(hr == S_OK, "got %#lx\n", hr); + memset(&st, 0xff, sizeof(st)); hr = ITask_GetNextRunTime(task, &st); ok(hr == S_OK, "got %#lx\n", hr); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1660
On Fri Dec 2 15:34:34 2022 +0000, **** wrote:
Dmitry Timoshkov replied on the mailing list: ``` "Sven Baars (@sbaars)" <wine(a)gitlab.winehq.org> wrote:
On Fri Dec 2 14:47:58 2022 +0000, **** wrote:
Dmitry Timoshkov replied on the mailing list: \`\`\` "Sven Baars (@sbaars)" <wine(a)gitlab.winehq.org> wrote:
On Fri Dec 2 14:16:59 2022 +0000, **** wrote:
Dmitry Timoshkov replied on the mailing list: \`\`\` Sven Baars <wine(a)gitlab.winehq.org> wrote:
- if (CompareFileTime(&begin_ft, &trigger_ft) <= 0 && CompareFileTime(&trigger_ft, &end_ft) < 0) + if (CompareFileTime(&trigger_ft, ¤t_ft) = 0) { if (!have_best_time || CompareFileTime(&trigger_ft, &best_ft) < 0) { Removing the trigger end time check doesn't look right. -- Dmitry. \`\`\` Hi Dmitry,
Thanks for the reply. I don't see how begin_ft and end_ft are relevant for this trigger. It happens exactly once, at the "Start" time and "Begin" day.
Having an "End" day before the "Begin" day is invalid for all triggers. I could add a patch that checks for that. Obviosuly the trigger shouldn't fire outside of begin/end time, so changing the trigger time bounds check doesn't look right. At best, that should be a separate patch with a bunch of tests. -- Dmitry. \`\`\` I submitted a v2 that addresses your concern. Looking at it again I don't understand the bounds check change. beging_ft already accounts for current_ft earlier in the code. Can you leave it as it is now? -- Dmitry.
Ah, I missed that, sorry. I left it unchanged now.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/1660#note_18447
participants (2)
-
Sven Baars -
Sven Baars (＠sbaars)