3
\$\begingroup\$

I wrote a simple tool to allow to daemonize any process under Microsoft Windows.

The goal is to push any process into the background with no questions asked. That includes GUI applications, which means that the main window will not be created...or at least not shown. For console applications it will simply detach the process from the current console and not open another one. Usage is simple:

daemonize.exe PROGRAM [ARGUMENTS]

So let's cut to the code:

#include <stdio.h>
#include <string.h>

#include <windows.h>

// The important things to note and know:
//
//  * Only works with ASCII paths and filenames.
//  * Quotes all arguments regardless of content.
//  * SW_HIDE in STARTUPINFO is needed for GUI applications
//  * DETACHED_PROCESS in CreateProcessA is needed for command line applications
//

int main(int argc, char *argv[]) {
    if (argc <= 1) {
        printf("daemonize.exe COMMAND [ARGUMENTS]");
        return 1;
    }

    char* command = argv[1];
    char* arguments = NULL;

    if (argc > 2) {
        // We only need to concatenate arguments if there
        // actually are arguments.
        int idx;

        // Calculate the length of all arguments
        size_t argLength = 0;
        for (idx = 1; idx < argc; idx++) {
            argLength += strlen(argv[idx] + 2 + 1); // One for the space, two for the quotes.
        }
        argLength--; // Strip the last space.

        // Reserve some memory and NULL the new string.
        //The + 1 is the space for the last null character.
        arguments = (char*)malloc(sizeof(char) * (argLength + 1));
        arguments[0] = 0;

        // Now concatenate the arguments.
        for (idx = 1; idx < argc; idx++) {
            strcat(arguments, "\"");
            strcat(arguments, argv[idx]);
            strcat(arguments, "\"");
            if (idx < argc - 1) {
                strcat(arguments, " ");
            }
        }
    }

    STARTUPINFO startInfo;
    PROCESS_INFORMATION process;

    ZeroMemory(&startInfo, sizeof(startInfo));
    ZeroMemory(&process, sizeof(process));

    startInfo.cb = sizeof(startInfo);

    // Tell the system to use/honor the ShowWindow flags.
    startInfo.dwFlags = STARTF_USESHOWWINDOW;
    // Tell the system that the main window of the process should be hidden.
    startInfo.wShowWindow = SW_HIDE;

    if (!CreateProcessA(
            command,            // application name
            arguments,            // command line arguments
            NULL,                // process attributes
            NULL,                // thread attributes
            FALSE,                // inherit (file) handles
            // Detach the process from the current console.
            DETACHED_PROCESS,   // creation flags
            NULL,                // environment
            NULL,                // current directory
            &startInfo,            // startup info
            &process)            // process information
        ) {

        printf("Creation of the process failed, trying to fetch error message...\n");

        long lastError = GetLastError();
        char* message = NULL;

        FormatMessageA(
            FORMAT_MESSAGE_ALLOCATE_BUFFER |
            FORMAT_MESSAGE_ARGUMENT_ARRAY |
            FORMAT_MESSAGE_FROM_SYSTEM |
            FORMAT_MESSAGE_IGNORE_INSERTS, // Flags
            NULL,               // source
            lastError,          // message identifier
            0,                  // language id
            (LPSTR)&message,   // message buffer
            0,                  // size of the message
            NULL                // arguments
        );

        printf(message);

        return 1;
    }

    return 0;
}
\$\endgroup\$
4
  • \$\begingroup\$ The Windows analogue of a daemon is a service. \$\endgroup\$ – 200_success Jan 7 '14 at 23:39
  • \$\begingroup\$ @200_success: The idea is/was that the process is bound against the session of the user. F.e. redshift which would always require an open cmd window if launched via an "start all my applications on login" script. \$\endgroup\$ – Bobby Jan 8 '14 at 8:14
  • \$\begingroup\$ What's the question? \$\endgroup\$ – ChrisW Jan 11 '14 at 22:54
  • \$\begingroup\$ @ChrisW: No direct question, I think I did everything to my best ability and I'm not sure if I could to anything else, I just want a code review to be sure, because this is one of the few pieces of C/C++ code that I ever wrote. \$\endgroup\$ – Bobby Jan 12 '14 at 11:06
2
\$\begingroup\$

It looks good: very good, assuming it works (i.e. has the run-time behaviour that you want: I don't know the effect of calling CreateProcess in that way).

You don't free(arguments) but that doesn't matter because memory will be reclaimed by the O/S when the process exits. Similarly, in theory I'd expect an API call to release message.

I don't see why you need the (LPSTR) cast. Using a cast can hide compiler warnings/errors, which should be fixed instead of their causing errors at run-time.

Instead of int idx; it's better to not define a local variable until the moment at which you initialize it: instead you could say, for (int idx = 1; idx < argc; idx++).

Also, shouldn't it be for (int idx = 2; idx < argc; idx++)? Because, idx=1 is the 'command' parameter, not the first 'argument' parameter.


If when you say "C/C++" you mean "C++", then it's more conventional these days to use std::string instead of C-style strings (but this is an unusually short program; perhaps although I don't know why you want to avoid the hassle of linking to the STL, especially if you know enough to avoid the many pitfalls of C-style string manipulation, which you seem to have).

For example, using std::string you could write the following, which is simpler than the string-manipulation code you have at the moment:

std::string s;
for (int idx = 1; idx < argc; idx++)
{
    s += "\"" + argv[idx] + "\"";
    if (idx < argc - 1) {
        s += " ";
    }
}
char* arguments = strdup(s.c_str());

The reason for strdup in the above is that MSDN says ...

The Unicode version of this function, CreateProcessW, can modify the contents of this string. Therefore, this parameter cannot be a pointer to read-only memory (such as a const variable or a literal string). If this parameter is a constant string, the function may cause an access violation.

... which helps to explain why arguments must be char* not const char*. Sometimes you can ignore these (the windows API sometimes doesn't say its parameters are const when in fact they are), but doing so may be unsafe bad practice (so I use strdup above -- note that strdup'ed memory should be free'ed).

\$\endgroup\$
4
  • 1
    \$\begingroup\$ The question is in C, not C++. C uses C-style strings. \$\endgroup\$ – 200_success Jan 12 '14 at 16:52
  • \$\begingroup\$ The OP called it "C/C++ code" in a comment. \$\endgroup\$ – ChrisW Jan 12 '14 at 17:07
  • 1
    \$\begingroup\$ You misinterpreted the comment. The OP is just saying that has little experience in either C or C++ (i.e. usually programs in Java). The question is still tagged c. \$\endgroup\$ – 200_success Jan 12 '14 at 17:16
  • \$\begingroup\$ @200_success Whatever. The first (original) part of the answer is applicable to C. The 2nd part is an example/comparison with C++. C programmers used to use C++ compilers as "a better C": to compile their C code, only gradually adding C++-specific features as they learned them. \$\endgroup\$ – ChrisW Jan 12 '14 at 17:21
1
\$\begingroup\$

I know that saying this misses the point of the question, but I couldn't help myself from remarking this:

Separating the string creation and allocation for arguments into it's own function shaves about thirty lines from your main function (it does, however add, rather than substract, from the final file size). It's mainly in the interests of readability. I append a proof-of-concept implementation.

char* quoteAndSerialize(int count, const char **vector)
{
/*
    Concatenate the first count elements of vector into a single string.

    The elements are quoted before concatenation, and each pair of consecutive
    elements is separated by a space character. The resulting string is
    zero-terminated. If the pointer to the resulting string is lost before
    deallocation, memory leaks will ocurr.
*/
    int finalSize;
    char* string;

    finalSize = quoteAndSerialize_finalSize(count, vector);
    string = (char*) malloc(sizeof(char) * finalSize);
    string[0] = 0;

    for (int i = 0; i < count; i++) {
        strcat(arguments, "\"");
        strcat(arguments, vector[i]);
        strcat(arguments, "\"");
        if (i < count - 1) {
            strcat(arguments, " ");
        }
    }
    return string;
}

int quoteAndSerialize_finalSize(int count, const char **vector)
{
    int finalSize = 0;

    //add together the length of all arguments
    for (int i = 0; i < count; i++)
    {
        finalSize += strlen(vector[i]);
    }

    // for every argument, there will be two \" characters surrounding it
    finalSize += 2 * count;

    // for every argument after the first, there will be a space separating
    // it from the previous one
    if (count != 0) finalSize += count - 1;

    // since strings are zero-terminated:
    finalSize += 1;

    return finalSize;
}

This allows for something like:

(...)

    char* command = argv[1];
    char* arguments = NULL;

    // create a null-terminated string containing all arguments, each surrounded
    // by quotes, and separated by spaces
    arguments = quoteAndSerialize(argc - 2, argv + 2)

    STARTUPINFO startInfo;
    PROCESS_INFORMATION process;


(...)
\$\endgroup\$
4
  • \$\begingroup\$ Welcome to Code Review! I recommend that you add more context to your post: while your post isn't entirely a code dump, all it is doing is saying that you've reduced the amount of lines in the code, and you even said that it "misses the point of the question". While you don't have to delete your answer, I recommend that you add more to it: why should the OP do what you have done, other than that it cuts down on some lines? \$\endgroup\$ – SirPython Sep 25 '15 at 23:24
  • \$\begingroup\$ Thank you, SirPython, for your quick reply. My post does miss the point of the question: the question is whether the presented code is correct and it does what the OP says it does. The content of my reply was relevant to the style and readability of the code, not to it's pourpose or correctness; this is why I had qualms about posting. The reason I did post is because I spend a good five minutes looking at the code before I figured that the entire block after the first 'if' wasn't relevant to the problem, instead of the five seconds I could've. \$\endgroup\$ – Paul Sep 25 '15 at 23:41
  • \$\begingroup\$ Readability and style reviews are perfectly acceptable here. What @SirPython was saying is that you should state why you feel this would be better than the current version. \$\endgroup\$ – user34073 Sep 26 '15 at 0:51
  • \$\begingroup\$ It does, as a matter of fact, not miss the point of the question. Now in hindsight, I'm rather surprised that I did not do this, so yes, your answer is perfectly fine and on-topic. The processing of the arguments should be in it's own function. \$\endgroup\$ – Bobby Sep 26 '15 at 9:26

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.