Re: [PATCH v2] taskmgr/endproc.c: Implement ProcessPage_OnEndProcessTree()

[Bruno Jesus]

Hi, just a variable type change and minor style suggestions, some you
can accept or ignore.
> ...
>
> +static BOOL process_list_append(process_list *list, DWORD id)
> +{
> - INT *temp;
> - if (list->count == list->size)
> {
> -
>    GetLastErrorText(wstrErrorText, sizeof(wstrErrorText)/sizeof(WCHAR));
> -
>    MessageBoxW(hMainWnd, wstrErrorText,wszUnable2Terminate, MB_OK|MB_ICONSTOP);
> -
>    return;
> -
>    list->size *= 2;
> -
>    temp = HeapReAlloc(GetProcessHeap(), 0, list->pid, list->size * sizeof(*list->pid));
> -
>    if(!temp)
> -
>        return FALSE;
> -
> -
>    list->pid = temp;
> }
> - list->pid[list->count++] = id;
> - return TRUE;
>
> +}

Please use an empty line to separate variable declaration from code.
Change "INT *temp" to "DWORD *temp" so it matches the same type from
list->pid. You can also reduce the scope of the temp variable. There
are 4 uses of "if(", please change to "if (".
> - if (!TerminateProcess(hProcess, 0))
>
> +static void free_process_list(process_list *list)
> +{
> - HeapFree(GetProcessHeap(), 0, list->pid);
>
> +}
> -
>
> +static void enum_process_children(HANDLE snapshot, process_list *list, DWORD pid)
> +{
> - PROCESSENTRY32 entry;
> - SIZE_T start, end, i;
> -
> - start = list->count;
> - entry.dwSize = sizeof(entry);
> -
> - if(!Process32First(snapshot, &entry))
> -
>    return;
> -
> - do
> - {
> -
>    if(entry.th32ParentProcessID == pid)
> -
>    {
> -
>        if(!process_list_append(list, entry.th32ProcessID))
> -
>        {
> -
>            free_process_list(list);
> -
>            return;
> -
>        }
> -
>    }
> - } while (Process32Next(snapshot, &entry));
> -
> - end = list->count;
> - for (i = start; i < end; ++i)
> {
> -
>    GetLastErrorText(wstrErrorText, sizeof(wstrErrorText)/sizeof(WCHAR));
> -
>    MessageBoxW(hMainWnd, wstrErrorText,wszUnable2Terminate, MB_OK|MB_ICONSTOP);
> -
>    enum_process_children(snapshot, list, list->pid[i]);
> }

You can drop the { } since there is a single line now, but it is just
a suggestion. Other style suggestion is to merge the end attribution
inside the for like "for (i = start, end = list-> count; ...".
> -
> - CloseHandle(hProcess);
>
> }
>
> void ProcessPage_OnEndProcessTree(void)
> @@ -100,11 +171,14 @@ void ProcessPage_OnEndProcessTree(void)
>      DWORD            dwProcessId;
>      HANDLE           hProcess;
>      WCHAR            wstrErrorText[256];
> -
>
> process_list     list;
> -
>
> SIZE_T           i;
> -
>
> HANDLE           snapshot;
>
> load_message_strings();
>
> Count = SendMessageW(hProcessPageListCtrl, LVM_GETITEMCOUNT, 0, 0);
> - for (Index=0; Index<Count; Index++)
> - for (Index = 0; Index < Count; Index++)
> {
>     lvitem.mask = LVIF_STATE;
>     lvitem.stateMask = LVIS_SELECTED;
>
> @@ -116,7 +190,6 @@ void ProcessPage_OnEndProcessTree(void)
>          if (lvitem.state & LVIS_SELECTED)
>              break;
>      }
> - Count = SendMessageW(hProcessPageListCtrl, LVM_GETSELECTEDCOUNT, 0, 0);

Please don't remove the empty line to separate the code from the "for" block.
> ...

On Sat, Apr 29, 2017 at 7:08 PM, Akarsha Sehwag
 wrote:
> Fixes https://bugs.winehq.org/show_bug.cgi?id=39640
>
> Implemented ProcessPage_OnEndProcessTree(). It now kills all the child processes along with the specified process.
> Signed-off-by: Akarsha Sehwag
>
> programs/taskmgr/endproc.c | 125 +++++++++++++++++++++++++++++++++++++--------
>  1 file changed, 105 insertions(+), 20 deletions(-)
>
> diff --git a/programs/taskmgr/endproc.c b/programs/taskmgr/endproc.c
> index 89c2d7b..60df0d7 100644
> --- a/programs/taskmgr/endproc.c
> +++ b/programs/taskmgr/endproc.c
> @@ -31,11 +31,18 @@
>  #include "wine/unicode.h"
>  #include "taskmgr.h"
>  #include "perfdata.h"
> +#include "tlhelp32.h"
>
> static WCHAR wszWarnMsg[511];
>  static WCHAR wszWarnTitle[255];
>  static WCHAR wszUnable2Terminate[255];
>
> +typedef struct process_list {
> - DWORD       *pid;
> - SIZE_T      count;      /* index to maintain the last entry of the array */
> - SIZE_T      size;
>
> +} process_list;
> -
>
> static void load_message_strings(void)
>  {
>      LoadStringW(hInst, IDS_TERMINATE_MESSAGE, wszWarnMsg, sizeof(wszWarnMsg)/sizeof(WCHAR));
> @@ -43,6 +50,22 @@ static void load_message_strings(void)
>      LoadStringW(hInst, IDS_WARNING_TITLE, wszWarnTitle, sizeof(wszWarnTitle)/sizeof(WCHAR));
>  }
>
> +static void kill_process(HANDLE hProcess, WCHAR *wstrErrorText)
> +{
> - if (!hProcess)
> - {
> -
>    GetLastErrorText(wstrErrorText, sizeof(wstrErrorText)/sizeof(WCHAR));
> -
>    MessageBoxW(hMainWnd, wstrErrorText, wszUnable2Terminate, MB_OK|MB_ICONSTOP);
> -
>    return;
> - }
> -
> - if (!TerminateProcess(hProcess, 0))
> - {
> -
>    GetLastErrorText(wstrErrorText, sizeof(wstrErrorText)/sizeof(WCHAR));
> -
>    MessageBoxW(hMainWnd, wstrErrorText, wszUnable2Terminate, MB_OK|MB_ICONSTOP);
> - }
>
> +}
> -
>
> void ProcessPage_OnEndProcess(void)
>  {
>      LVITEMW          lvitem;
> @@ -54,7 +77,7 @@ void ProcessPage_OnEndProcess(void)
>      load_message_strings();
>  Count = SendMessageW(hProcessPageListCtrl, LVM_GETITEMCOUNT, 0, 0);
> - for (Index=0; Index<Count; Index++)
> - for (Index=0; Index < Count; Index++)
> {
>     lvitem.mask = LVIF_STATE;
>     lvitem.stateMask = LVIS_SELECTED;
>
> @@ -77,20 +100,68 @@ void ProcessPage_OnEndProcess(void)
>  hProcess = OpenProcess(PROCESS_TERMINATE, FALSE, dwProcessId);
>
> - if (!hProcess)
> - kill_process(hProcess, wstrErrorText);
> - CloseHandle(hProcess);
>
> +}
> -
>
> +static BOOL init_process_list(process_list *list)
> +{
> - list->size = 4;            /* initialise size with 4. Will increase if necessary */
> - list->pid = HeapAlloc(GetProcessHeap(), 0, list->size * sizeof(*list->pid));
> - list->count = 0;
> -
> - return list->pid != NULL;
>
> +}
> -
>
> +static BOOL process_list_append(process_list *list, DWORD id)
> +{
> - INT *temp;
> - if (list->count == list->size)
> {
> -
>    GetLastErrorText(wstrErrorText, sizeof(wstrErrorText)/sizeof(WCHAR));
> -
>    MessageBoxW(hMainWnd, wstrErrorText,wszUnable2Terminate, MB_OK|MB_ICONSTOP);
> -
>    return;
> -
>    list->size *= 2;
> -
>    temp = HeapReAlloc(GetProcessHeap(), 0, list->pid, list->size * sizeof(*list->pid));
> -
>    if(!temp)
> -
>        return FALSE;
> -
> -
>    list->pid = temp;
> }
> - list->pid[list->count++] = id;
> - return TRUE;
>
> +}
> - if (!TerminateProcess(hProcess, 0))
>
> +static void free_process_list(process_list *list)
> +{
> - HeapFree(GetProcessHeap(), 0, list->pid);
>
> +}
> -
>
> +static void enum_process_children(HANDLE snapshot, process_list *list, DWORD pid)
> +{
> - PROCESSENTRY32 entry;
> - SIZE_T start, end, i;
> -
> - start = list->count;
> - entry.dwSize = sizeof(entry);
> -
> - if(!Process32First(snapshot, &entry))
> -
>    return;
> -
> - do
> - {
> -
>    if(entry.th32ParentProcessID == pid)
> -
>    {
> -
>        if(!process_list_append(list, entry.th32ProcessID))
> -
>        {
> -
>            free_process_list(list);
> -
>            return;
> -
>        }
> -
>    }
> - } while (Process32Next(snapshot, &entry));
> -
> - end = list->count;
> - for (i = start; i < end; ++i)
> {
> -
>    GetLastErrorText(wstrErrorText, sizeof(wstrErrorText)/sizeof(WCHAR));
> -
>    MessageBoxW(hMainWnd, wstrErrorText,wszUnable2Terminate, MB_OK|MB_ICONSTOP);
> -
>    enum_process_children(snapshot, list, list->pid[i]);
> }
> -
> - CloseHandle(hProcess);
>
> }
>
> void ProcessPage_OnEndProcessTree(void)
> @@ -100,11 +171,14 @@ void ProcessPage_OnEndProcessTree(void)
>      DWORD            dwProcessId;
>      HANDLE           hProcess;
>      WCHAR            wstrErrorText[256];
> -
>
> process_list     list;
> -
>
> SIZE_T           i;
> -
>
> HANDLE           snapshot;
>
> load_message_strings();
>
> Count = SendMessageW(hProcessPageListCtrl, LVM_GETITEMCOUNT, 0, 0);
> - for (Index=0; Index<Count; Index++)
> - for (Index = 0; Index < Count; Index++)
> {
>     lvitem.mask = LVIF_STATE;
>     lvitem.stateMask = LVIS_SELECTED;
>
> @@ -116,7 +190,6 @@ void ProcessPage_OnEndProcessTree(void)
>          if (lvitem.state & LVIS_SELECTED)
>              break;
>      }
> - Count = SendMessageW(hProcessPageListCtrl, LVM_GETSELECTEDCOUNT, 0, 0);
>    dwProcessId = PerfDataGetProcessId(Index);
>    if ((Count != 1) || (dwProcessId == 0))
>
> @@ -125,20 +198,32 @@ void ProcessPage_OnEndProcessTree(void)
>      if (MessageBoxW(hMainWnd, wszWarnMsg, wszWarnTitle, MB_YESNO|MB_ICONWARNING) != IDYES)
>          return;
> - hProcess = OpenProcess(PROCESS_TERMINATE, FALSE, dwProcessId);
> - if (!init_process_list(&list))
> -
>    return;
>
> - if (!hProcess)
> - if (!process_list_append(&list, dwProcessId))
> {
> -
>    GetLastErrorText(wstrErrorText, sizeof(wstrErrorText)/sizeof(WCHAR));
> -
>    MessageBoxW(hMainWnd, wstrErrorText,wszUnable2Terminate, MB_OK|MB_ICONSTOP);
> -
>    free_process_list(&list);
>    return;
> }
> - if (!TerminateProcess(hProcess, 0))
> - snapshot = CreateToolhelp32Snapshot(TH32CS_SNAPPROCESS, 0);
> -
> - if (snapshot == INVALID_HANDLE_VALUE)
> {
> -
>    GetLastErrorText(wstrErrorText, sizeof(wstrErrorText)/sizeof(WCHAR));
> -
>    MessageBoxW(hMainWnd, wstrErrorText,wszUnable2Terminate, MB_OK|MB_ICONSTOP);
> -
>    free_process_list(&list);
> -
>    return;
> }
> - enum_process_children(snapshot, &list, dwProcessId);
> -
> - CloseHandle(snapshot);
> - CloseHandle(hProcess);
> - for (i = 0; i < list.count; ++i)
> - {
> -
>    hProcess = OpenProcess(PROCESS_TERMINATE, FALSE, list.pid[i]);
> -
> -
>    kill_process(hProcess, wstrErrorText);
> -
>    CloseHandle(hProcess);
> - }
> - free_process_list(&list);
> }
>
> 2.9.3