5
\$\begingroup\$

(Also, see the next iteration.)

I have this small program for terminating processes via their respective process image names (.exe files):

#include <stdio.h>
#include <windows.h>
#include <TlHelp32.h>

int main(int argc, char* argv[]) {
    if (argc != 2) {
        puts("processkiller.exe PROCESS_NAME");
        return EXIT_SUCCESS;
    }

    PROCESSENTRY32 entry;
    entry.dwSize = sizeof(PROCESSENTRY32);
    HANDLE snapshot = CreateToolhelp32Snapshot(TH32CS_SNAPPROCESS, NULL);

    if (Process32First(snapshot, &entry) == TRUE) {
        while (Process32Next(snapshot, &entry) == TRUE) {
            if (strcmp(entry.szExeFile, argv[1]) == 0) {
                HANDLE hProcess = OpenProcess(PROCESS_ALL_ACCESS, 
                                              FALSE, 
                                              entry.th32ProcessID);
                TerminateProcess(hProcess, 0);
                CloseHandle(hProcess);
            }
        }
    }

    CloseHandle(snapshot);
    return EXIT_SUCCESS;
}

Note that you need to run processkiller.exe in administrator mode in order to actually terminate the requested processes.

Critique request

Please tell me anything that comes to mind.

\$\endgroup\$
4
\$\begingroup\$

There is a subtle bug:

Process32First(snapshot, &entry)

already fills entry with the information about the first process in the snapshot. Your code misses that entry because Process32Next() is called immediately. The loop structure should be

if (Process32First(snapshot, &entry) == TRUE) {
    do {
        // ... do something with `entry`...
        
    } while (Process32Next(snapshot, &entry) == TRUE);
}

instead. Other things that come into my mind:

  • If the program is called with the wrong number of arguments then the error/usage message should be printed to the standard error and the program should terminate with a non-zero exit code, e.g. EXIT_FAILURE.
  • The return value of CreateToolhelp32Snapshot() is not checked.
  • PROCESS_ALL_ACCESS is not needed in the call to OpenProcess, only PROCESS_TERMINATE.
  • The return values of OpenProcess() and TerminateProcess() are not checked. I would expect a diagnostic message if they fail. In particular, TerminateProcess() and CloseHandle() should only be called if OpenProcess() succeeded.
  • It may be a matter of taste, but == TRUE can be omitted when checking a boolean condition.
  • As a user of this tool I would expect some feedback to see if a matching process was found, and how many processes were killed.
\$\endgroup\$
1
  • \$\begingroup\$ Thanks for debugging the loop! Nice answer! \$\endgroup\$ – coderodde Aug 21 '20 at 9:00

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.