Skip to main content
added 228 characters in body
Source Link
Martin R
  • 24.2k
  • 2
  • 38
  • 96

Some remarks with respect to the get_base_name() function:

  • size_t vs int: You correctly start with

    size_t arg_len = strlen(arg); 

    but then pass arg_len to get_last_backslash_index() which takes an int argument. Depending on the strictness of your compiler this can cause a “Implicit conversion loses integer precision” warning. I suggest to use size_t consequently for string length.

  • The cast in

    char* carr = (char*)calloc(return_char_array_len, sizeof(char)); 

    is not needed and generally not recommended, see for example Do I cast the result of malloc? on Stack Overflow.

  • sizeof(char) is always equal to one, therefore it can be shortened to

    char* carr = malloc(return_char_array_len); 
  • Here

    carr[return_char_array_len - 1] = NULL; 

    a pointer value is assigned to char (which is an integral type). The right-hand side should be 0 or '\0' to avoid compiler warnings.

  • The implementation can be shortened if you take advantage of standardMicrosoft C runtime library functions like strrchr()strrchr() and strdup()_strdup():

    static char* get_base_name(const char* const arg) { const char *backslash_char_ptr = strrchr(arg, '\\'); return strdup_strdup(backslash_char_ptr == NULL ? arg : backslash_char_ptr + 1); } 
  • I would perhaps call the function something like copy_base_name() to make it more obvious that the caller is responsible for releasing the memory.

  • As an alternative you can use existing functions from the Microsoft C runtime like splitpath, or Shell Path Handling Functions like PathStripPath().

Some more remarks:

  • No error message is printed if terminating a process failed.
  • The process handle is closed only if terminating the process succeeded.

I would perhaps do it like this:

HANDLE hProcess = OpenProcess(...); if (hProcess == NULL) { // Print error message } else { if (TerminateProcess(hProcess, 0)) { totalProcessesTerminated++; printf("Terminated process ID %d\n", entry.th32ProcessID); } else { // Print error message } if (!CloseHandle(hProcess)) { // Print error message } } 

In all error situations it would be useful to print the actual reason for the failure (e.g. “access denied”). This can be achieved with GetLastError() and FormatMessage(), see for example How to get the error message from the error code returned by GetLastError()? on Stack Overflow.

Some remarks with respect to the get_base_name() function:

  • size_t vs int: You correctly start with

    size_t arg_len = strlen(arg); 

    but then pass arg_len to get_last_backslash_index() which takes an int argument. Depending on the strictness of your compiler this can cause a “Implicit conversion loses integer precision” warning. I suggest to use size_t consequently for string length.

  • The cast in

    char* carr = (char*)calloc(return_char_array_len, sizeof(char)); 

    is not needed and generally not recommended, see for example Do I cast the result of malloc? on Stack Overflow.

  • sizeof(char) is always equal to one, therefore it can be shortened to

    char* carr = malloc(return_char_array_len); 
  • Here

    carr[return_char_array_len - 1] = NULL; 

    a pointer value is assigned to char (which is an integral type). The right-hand side should be 0 or '\0' to avoid compiler warnings.

  • The implementation can be shortened if you take advantage of standard C library functions like strrchr() and strdup():

    static char* get_base_name(const char* const arg) { const char *backslash_char_ptr = strrchr(arg, '\\'); return strdup(backslash_char_ptr == NULL ? arg : backslash_char_ptr + 1); } 
  • I would perhaps call the function something like copy_base_name() to make it more obvious that the caller is responsible for releasing the memory.

  • As an alternative you can use existing functions from the Microsoft C runtime like splitpath, or Shell Path Handling Functions like PathStripPath().

Some more remarks:

  • No error message is printed if terminating a process failed.
  • The process handle is closed only if terminating the process succeeded.

I would perhaps do it like this:

HANDLE hProcess = OpenProcess(...); if (hProcess == NULL) { // Print error message } else { if (TerminateProcess(hProcess, 0)) { totalProcessesTerminated++; printf("Terminated process ID %d\n", entry.th32ProcessID); } else { // Print error message } if (!CloseHandle(hProcess)) { // Print error message } } 

In all error situations it would be useful to print the actual reason for the failure (e.g. “access denied”). This can be achieved with GetLastError() and FormatMessage(), see for example How to get the error message from the error code returned by GetLastError()? on Stack Overflow.

Some remarks with respect to the get_base_name() function:

  • size_t vs int: You correctly start with

    size_t arg_len = strlen(arg); 

    but then pass arg_len to get_last_backslash_index() which takes an int argument. Depending on the strictness of your compiler this can cause a “Implicit conversion loses integer precision” warning. I suggest to use size_t consequently for string length.

  • The cast in

    char* carr = (char*)calloc(return_char_array_len, sizeof(char)); 

    is not needed and generally not recommended, see for example Do I cast the result of malloc? on Stack Overflow.

  • sizeof(char) is always equal to one, therefore it can be shortened to

    char* carr = malloc(return_char_array_len); 
  • Here

    carr[return_char_array_len - 1] = NULL; 

    a pointer value is assigned to char (which is an integral type). The right-hand side should be 0 or '\0' to avoid compiler warnings.

  • The implementation can be shortened if you take advantage of Microsoft C runtime library functions like strrchr() and _strdup():

    static char* get_base_name(const char* const arg) { const char *backslash_char_ptr = strrchr(arg, '\\'); return _strdup(backslash_char_ptr == NULL ? arg : backslash_char_ptr + 1); } 
  • I would perhaps call the function something like copy_base_name() to make it more obvious that the caller is responsible for releasing the memory.

  • As an alternative you can use existing functions from the Microsoft C runtime like splitpath, or Shell Path Handling Functions like PathStripPath().

Some more remarks:

  • No error message is printed if terminating a process failed.
  • The process handle is closed only if terminating the process succeeded.

I would perhaps do it like this:

HANDLE hProcess = OpenProcess(...); if (hProcess == NULL) { // Print error message } else { if (TerminateProcess(hProcess, 0)) { totalProcessesTerminated++; printf("Terminated process ID %d\n", entry.th32ProcessID); } else { // Print error message } if (!CloseHandle(hProcess)) { // Print error message } } 

In all error situations it would be useful to print the actual reason for the failure (e.g. “access denied”). This can be achieved with GetLastError() and FormatMessage(), see for example How to get the error message from the error code returned by GetLastError()? on Stack Overflow.

deleted 39 characters in body
Source Link
Martin R
  • 24.2k
  • 2
  • 38
  • 96

Some remarks with respect to the get_base_name() function:

  • size_t vs int: You correctly start with

    size_t arg_len = strlen(arg); 

    but then pass arg_len to get_last_backslash_index() which takes an int argument. Depending on the strictness of your compiler this can cause a “Implicit conversion loses integer precision” warning. I suggest to use size_t consequently for string length.

  • The cast in

    char* carr = (char*)calloc(return_char_array_len, sizeof(char)); 

    is not needed and generally not recommended, see for example Do I cast the result of malloc? on Stack Overflow.

  • sizeof(char) is always equal to one, therefore it can be shortened to

    char* carr = malloc(return_char_array_len); 
  • Here

    carr[return_char_array_len - 1] = NULL; 

    a pointer value is assigned to char (which is an integral type). The right-hand side should be 0 or '\0' to avoid compiler warnings.

  • The implementation can be shortened if you take advantage of standard C library functions like strrchr() and strdup():

    static char* get_base_name(const char* const arg) { const char *backslash_char_ptr = strrchr(arg, '\\'); return strdup(backslash_char_ptr == NULL ? arg : backslash_char_ptr + 1); } 
  • I would perhaps call the function something like copy_base_name() to make it more obvious that the caller is responsible for releasing the memory.

  • As an alternative you can use existing functions from the Microsoft C runtime like splitpath, or Shell Path Handling Functions like PathStripPath().

Some more remarks:

  • No error message is printed if terminating a process failed.
  • The process handle is closed only if terminating the process succeeded.

I would perhaps do it like this:

HANDLE hProcess = OpenProcess(...); if (hProcess == NULL) { // Print error message } else { if (TerminateProcess(hProcess, 0)) { totalProcessesTerminated++; printf("Terminated process ID %d\n", entry.th32ProcessID); } else { // Print error message } if (!CloseHandle(hProcess)) { // Print error message } } 

In theall error situations it would be useful to print the actual reason for the failure (e.g. “access denied”). This can be achieved with GetLastError() and FormatMessage() to print the reason (e.g. “access denied”), see for example How to get the error message from the error code returned by GetLastError()? on Stack Overflow.

Some remarks with respect to the get_base_name() function:

  • size_t vs int: You correctly start with

    size_t arg_len = strlen(arg); 

    but then pass arg_len to get_last_backslash_index() which takes an int argument. Depending on the strictness of your compiler this can cause a “Implicit conversion loses integer precision” warning. I suggest to use size_t consequently for string length.

  • The cast in

    char* carr = (char*)calloc(return_char_array_len, sizeof(char)); 

    is not needed and generally not recommended, see for example Do I cast the result of malloc? on Stack Overflow.

  • sizeof(char) is always equal to one, therefore it can be shortened to

    char* carr = malloc(return_char_array_len); 
  • Here

    carr[return_char_array_len - 1] = NULL; 

    a pointer value is assigned to char (which is an integral type). The right-hand side should be 0 or '\0' to avoid compiler warnings.

  • The implementation can be shortened if you take advantage of standard C library functions like strrchr() and strdup():

    static char* get_base_name(const char* const arg) { const char *backslash_char_ptr = strrchr(arg, '\\'); return strdup(backslash_char_ptr == NULL ? arg : backslash_char_ptr + 1); } 
  • I would perhaps call the function something like copy_base_name() to make it more obvious that the caller is responsible for releasing the memory.

  • As an alternative you can use existing functions from the Microsoft C runtime like splitpath, or Shell Path Handling Functions like PathStripPath().

Some more remarks:

  • No error message is printed if terminating a process failed.
  • The process handle is closed only if terminating the process succeeded.

I would perhaps do it like this:

HANDLE hProcess = OpenProcess(...); if (hProcess == NULL) { // Print error message } else { if (TerminateProcess(hProcess, 0)) { totalProcessesTerminated++; printf("Terminated process ID %d\n", entry.th32ProcessID); } else { // Print error message } if (!CloseHandle(hProcess)) { // Print error message } } 

In the error situations it would be useful to print the actual reason for failure (e.g. “access denied”). This can be achieved with GetLastError() and FormatMessage() to print the reason (e.g. “access denied”), see for example How to get the error message from the error code returned by GetLastError()? on Stack Overflow.

Some remarks with respect to the get_base_name() function:

  • size_t vs int: You correctly start with

    size_t arg_len = strlen(arg); 

    but then pass arg_len to get_last_backslash_index() which takes an int argument. Depending on the strictness of your compiler this can cause a “Implicit conversion loses integer precision” warning. I suggest to use size_t consequently for string length.

  • The cast in

    char* carr = (char*)calloc(return_char_array_len, sizeof(char)); 

    is not needed and generally not recommended, see for example Do I cast the result of malloc? on Stack Overflow.

  • sizeof(char) is always equal to one, therefore it can be shortened to

    char* carr = malloc(return_char_array_len); 
  • Here

    carr[return_char_array_len - 1] = NULL; 

    a pointer value is assigned to char (which is an integral type). The right-hand side should be 0 or '\0' to avoid compiler warnings.

  • The implementation can be shortened if you take advantage of standard C library functions like strrchr() and strdup():

    static char* get_base_name(const char* const arg) { const char *backslash_char_ptr = strrchr(arg, '\\'); return strdup(backslash_char_ptr == NULL ? arg : backslash_char_ptr + 1); } 
  • I would perhaps call the function something like copy_base_name() to make it more obvious that the caller is responsible for releasing the memory.

  • As an alternative you can use existing functions from the Microsoft C runtime like splitpath, or Shell Path Handling Functions like PathStripPath().

Some more remarks:

  • No error message is printed if terminating a process failed.
  • The process handle is closed only if terminating the process succeeded.

I would perhaps do it like this:

HANDLE hProcess = OpenProcess(...); if (hProcess == NULL) { // Print error message } else { if (TerminateProcess(hProcess, 0)) { totalProcessesTerminated++; printf("Terminated process ID %d\n", entry.th32ProcessID); } else { // Print error message } if (!CloseHandle(hProcess)) { // Print error message } } 

In all error situations it would be useful to print the actual reason for the failure (e.g. “access denied”). This can be achieved with GetLastError() and FormatMessage(), see for example How to get the error message from the error code returned by GetLastError()? on Stack Overflow.

added 165 characters in body
Source Link
Martin R
  • 24.2k
  • 2
  • 38
  • 96

Some remarks with respect to the get_base_name() function:

  • size_t vs int: You correctly start with

    size_t arg_len = strlen(arg); 

    but then pass arg_len to get_last_backslash_index() which takes an int argument. Depending on the strictness of your compiler this can cause a “Implicit conversion loses integer precision” warning. I suggest to use size_t consequently for string length.

  • The cast in

    char* carr = (char*)calloc(return_char_array_len, sizeof(char)); 

    is not needed and generally not recommended, see for example Do I cast the result of malloc? on Stack Overflow.

  • sizeof(char) is always equal to one, therefore it can be shortened to

    char* carr = malloc(return_char_array_len); 
  • Here

    carr[return_char_array_len - 1] = NULL; 

    a pointer value is assigned to char (which is an integral type). The right-hand side should be 0 or '\0' to avoid compiler warnings.

  • The implementation can be shortened if you take advantage of standard C library functions like strrchr() and strdup():

    static char* get_base_name(const char* const arg) { const char *backslash_char_ptr = strrchr(arg, '\\'); return strdup(backslash_char_ptr == NULL ? arg : backslash_char_ptr + 1); } 
  • I would perhaps call the function something like copy_base_name() to make it more obvious that the caller is responsible for releasing the memory.

  • As an alternative you can use existing functions from the Microsoft C runtime like splitpath, or Shell Path Handling Functions like PathStripPath().

Some more remarks:

  • No error message is printed if terminating a process failed.
  • The process handle is closed only if terminating the process succeeded.

I would perhaps do it like this:

HANDLE hProcess = OpenProcess(...); if (hProcess == NULL) { // Print error message } else { if (TerminateProcess(hProcess, 0)) { totalProcessesTerminated++; printf("Terminated process ID %d\n", entry.th32ProcessID); } else { // Print error message } if (!CloseHandle(hProcess)) { // Print error message } } 

In the error situations it would be useful to print the actual reason for failure (e.g. “access denied”). This can be achieved with GetLastError() and FormatMessage() to print the reason (e.g. “access denied”), see for example How to get the error message from the error code returned by GetLastError()? on Stack Overflow.

Some remarks with respect to the get_base_name() function:

  • size_t vs int: You correctly start with

    size_t arg_len = strlen(arg); 

    but then pass arg_len to get_last_backslash_index() which takes an int argument. Depending on the strictness of your compiler this can cause a “Implicit conversion loses integer precision” warning. I suggest to use size_t consequently for string length.

  • The cast in

    char* carr = (char*)calloc(return_char_array_len, sizeof(char)); 

    is not needed and generally not recommended, see for example Do I cast the result of malloc? on Stack Overflow.

  • sizeof(char) is always equal to one, therefore it can be shortened to

    char* carr = malloc(return_char_array_len); 
  • Here

    carr[return_char_array_len - 1] = NULL; 

    a pointer value is assigned to char (which is an integral type). The right-hand side should be 0 or '\0' to avoid compiler warnings.

  • The implementation can be shortened if you take advantage of standard C library functions like strrchr() and strdup():

    static char* get_base_name(const char* const arg) { const char *backslash_char_ptr = strrchr(arg, '\\'); return strdup(backslash_char_ptr == NULL ? arg : backslash_char_ptr + 1); } 
  • I would perhaps call the function something like copy_base_name() to make it more obvious that the caller is responsible for releasing the memory.

  • As an alternative you can use existing functions from the Microsoft C runtime like splitpath, or Shell Path Handling Functions like PathStripPath().

Some more remarks:

  • No error message is printed if terminating a process failed.
  • The process handle is closed only if terminating the process succeeded.

Some remarks with respect to the get_base_name() function:

  • size_t vs int: You correctly start with

    size_t arg_len = strlen(arg); 

    but then pass arg_len to get_last_backslash_index() which takes an int argument. Depending on the strictness of your compiler this can cause a “Implicit conversion loses integer precision” warning. I suggest to use size_t consequently for string length.

  • The cast in

    char* carr = (char*)calloc(return_char_array_len, sizeof(char)); 

    is not needed and generally not recommended, see for example Do I cast the result of malloc? on Stack Overflow.

  • sizeof(char) is always equal to one, therefore it can be shortened to

    char* carr = malloc(return_char_array_len); 
  • Here

    carr[return_char_array_len - 1] = NULL; 

    a pointer value is assigned to char (which is an integral type). The right-hand side should be 0 or '\0' to avoid compiler warnings.

  • The implementation can be shortened if you take advantage of standard C library functions like strrchr() and strdup():

    static char* get_base_name(const char* const arg) { const char *backslash_char_ptr = strrchr(arg, '\\'); return strdup(backslash_char_ptr == NULL ? arg : backslash_char_ptr + 1); } 
  • I would perhaps call the function something like copy_base_name() to make it more obvious that the caller is responsible for releasing the memory.

  • As an alternative you can use existing functions from the Microsoft C runtime like splitpath, or Shell Path Handling Functions like PathStripPath().

Some more remarks:

  • No error message is printed if terminating a process failed.
  • The process handle is closed only if terminating the process succeeded.

I would perhaps do it like this:

HANDLE hProcess = OpenProcess(...); if (hProcess == NULL) { // Print error message } else { if (TerminateProcess(hProcess, 0)) { totalProcessesTerminated++; printf("Terminated process ID %d\n", entry.th32ProcessID); } else { // Print error message } if (!CloseHandle(hProcess)) { // Print error message } } 

In the error situations it would be useful to print the actual reason for failure (e.g. “access denied”). This can be achieved with GetLastError() and FormatMessage() to print the reason (e.g. “access denied”), see for example How to get the error message from the error code returned by GetLastError()? on Stack Overflow.

added 165 characters in body
Source Link
Martin R
  • 24.2k
  • 2
  • 38
  • 96
Loading
Source Link
Martin R
  • 24.2k
  • 2
  • 38
  • 96
Loading