2
\$\begingroup\$

Calling execv(3) (and similar functions) in C++ can be problematic. execv expects an array of const pointers to non-const char, and string literals are inherently const, so you have to jump through hoops to convert your literals to non-const char arrays. And of course in C++ you normally want to use vectors and strings instead of arrays.

Not finding any standard functions to handle this, I wrote two wrapper functions to make things easy. They seem to work correctly, in the ways I thought to test, but I’m fairly new to C++; more experienced developers may see any number of pitfalls I’ve missed.

My first version just used argv[0] as the path. Then I checked and found good reasons for path != argv[0] in some cases, so I overloaded it; one version is more general and one simplifies the call.

#include <cerrno>
#include <cstdio>
#include <cstring>
#include <string>
#include <vector>

#include <unistd.h>


int execv_cpp(const std::string &path,
              const std::vector<std::string> &argv);
int execv_cpp(const std::vector<std::string> &argv);


int main()
{
    // Test some failure modes.

    execv_cpp({});
    perror("execv() 1 failed");

    execv_cpp("/bin/ls", {});
    perror("execv() 2 failed");

    execv_cpp({"/foo/bar/baz"});
    perror("execv() 3 failed");

    execv_cpp({"/etc/passwd"});
    perror("execv() 4 failed");

    // Test valid calls.

    if (fork() == 0) {
        execv_cpp({"/bin/ls", "-lart"});
        perror("forked execv() unexpectedly failed");
        return 0;
    }

    usleep(50000);
    execv_cpp("/bin/ls", {"ls", "-lh"});
    perror("final execv() unexpectedly failed");

    return 0;
}


int execv_cpp(const std::string &path,
              const std::vector<std::string> &argv)
{
    /* Convert arguments to C-style and call execv. If it returns
     * (fails), clean up and pass return value to caller. */

    if (argv.size() == 0) {
        errno = EINVAL;
        return -1;
    }

    std::vector<char *> vec_cp;
    vec_cp.reserve(argv.size() + 1);
    for (auto s : argv)
        vec_cp.push_back(strdup(s.c_str()));
    vec_cp.push_back(NULL);

    int retval = execv(path.c_str(), vec_cp.data());

    int save_errno = errno;
    for (auto p : vec_cp)
        free(p);
    errno = save_errno;
    return retval;
}


int execv_cpp(const std::vector<std::string> &argv)
{
    /* Overloaded. Use first element as path for simpler call. */

    if (argv.size() == 0) {
        errno = EINVAL;
        return -1;
    }

    return execv_cpp(argv[0], argv);
}
\$\endgroup\$
4
\$\begingroup\$

Comments are not by any order of significance.

Which operating system standard?

A platform supporting C++ does not necessarily have an execve() call; that depends on <unistd.h>. Are you sure you're not making assumptions about what it contains?

Perhaps you aren't, but you need to double-check and document this.

Wrapping a single operating system call is questionable

If you've written this for your own use, since you execve() all the time like there's no tomorrow :-) ... then ok. Otherwise, developers would probably be hesitant to incorporate a idiosyncratic piece of code which handles exactly one out of dozens of system calls.

Also, even focusing on exec'ing another program, we have:

int execl(const char *path, const char *arg, ...);
int execlp(const char *file, const char *arg, ...);
int execle(const char *path, const char *arg, ...);
int execv(const char *path, char *const argv[]);
int execvp(const char *file, char *const argv[]);
int execvpe(const char *file, char *const argv[], char *const envp[]);

I'm not saying you need to implement a "Cpp-ified" version of all of these - on the contrary, it would probably be a bad idea - but you chose the less-general version.

Don't add utility/library functions to the global namespace

Application code uses (or rather, may use) the global namespace; library code uses a dedicated namespace. What you've written is supposed to be used repeatedly, not just once - so it's essentially library code, right? Put it in some namespace (e.g. util or unix etc.) A benefit of that is that you don't have to append the artificial name extension, i.e. unix::exec sounds nicer than execve_cpp.

Confusing parameters in your second variant

If I look at your second variant:

int execv_cpp(const std::vector<std::string> &argv);

It would not be clear to me whether the element of that vector should be the name of the binary I wish to execute. You may think you're making things more convenient for the client code, but people would rather type a few more characters than have to contend with semantic ambiguity.

If you're Cpp'ifying - do so for errors as well

In C++, passing invalid arguments typically causes an exception to be thrown; and you don't need to return anything, since you never return (as opposed to throwing an exception). Thus, for example:

void execv_cpp(const std::vector<std::string> &argv)
{
    if (argv.size() == 0) { 
        throw std::invalid_argument(
            "At least one argument must be provided, being the path of "
            "the binary to execute");
    execv_cpp(argv[0], argv);
}

but of course, you're making it easy for the calling code to get it wrong by being willing to take an empty argv in the first place.

Don't force people to use std::vector's

A vector is just one specific container. There's no reason to assume the calling code is using vectors; and you're not even actually using the vector - you're converting it into a C-style array.

in C++ you normally want to use vectors... instead of arrays.

Not true. You sometimes want to use them.

Don't force people to use std::strings

of course in C++ you normally want to use... strings...

Sometimes, sometimes not. An std::string is just one way - albeit the default way - to represent strings. Remember, it's in the library - it's not int he langue. And std::string involves memory allocation policy (stack & heap etc.) which might not be appropriate to everyone. You should be more flexible, e.g. by templating on the string type:

template<typename S>
void execv_cpp(const std::vector<S> &argv);

or by using string views (C++17, experimental in C++14).

Consider a C++'ish interface for a bounded number of arguments

If you know the number of arguments at compile time, you can just have the following:

template <typename Args...>
void execv_cpp(std::string_view binary, Args&&... arguments);

and use std::to_string(), or operator<< into a stringstream, on each of the arguments in a loop. That gives maximum flexibility to the calling code, and makes exec() ing a binary basically the same as calling an arbitrary function:

execv_cpp(my_binary, 1, "two", std::optional<int>(3), 4.0);

Now, true, in this case you would be using up memory for string conversion results, but in the fixed-number-of-arguments case it's acceptable, I would say.

No need to duplicate strings

In the answer you linked to, it specifically says it's safe to const_cast the char pointers. So no need for the strdup().

\$\endgroup\$
  • \$\begingroup\$ Thank you. I did specify the POSIX tag; anything POSIX-compliant should include execve and friends, no? I agree that wrapping all seven variations would be overkill; I chose to wrap the one that best fits my current project. Haven’t really studied namespaces yet, but will keep that advice in mind; as PEP 20 says, “Namespaces are one honking great idea”. The rest will have to wait until I’m less of a beginner. \$\endgroup\$ – Tom Zych Oct 7 '17 at 17:01

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.