DaanDeMeyer / reproc

A cross-platform (C99/C++11) process library
MIT License
552 stars 65 forks source link

No environment variable parameter #21

Closed lygstate closed 5 years ago

DaanDeMeyer commented 5 years ago

I was waiting with adding it as I wasn't sure if developers would actually need it. I'm definitely not opposed to adding it. Do you want to try adding it yourself? If not, I'll likely add it somewhere next week.

lygstate commented 5 years ago

@DaanDeMeyer I am trying add it, but look rather hard, environment variable are rather compilicated and need new data structure

DaanDeMeyer commented 5 years ago

My initial approach would be to add a const char *const *environment parameter to reproc_start. This can be passed without modification to execvpe and on Windows it seems like everything has to be concatenated together again before passing it to CreateProcess. However, instead of separating with spaces as is done when concatenating argv, when concatenating the environment block '\0' should be used as the delimiter (see lpEnviroment documentation in https://docs.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createprocessw).

EDIT: environment parameters would be passed to reproc_start as an array of null terminated strings of the format NAME=VALUE. reproc++'s convenience method for reproc_start could be extended to allow more convenient passing of environment variables as well.

lygstate commented 5 years ago

@DaanDeMeyer I would suggest the null/null terminated string const char environment instead conat char const *environment. The format are ${key}\0${value}\0${key}\0${value}\0\0. For simplify the memory allocation

lygstate commented 5 years ago

I've already implement a demo for parse from it, and concat to it, but for C++:) not for C. C handling this kinds of string are really complicated

// https://docs.microsoft.com/en-us/windows/win32/procthread/changing-environment-variables
inline std::map<std::string, std::string> parseEnvironmentString(const char* env) {
    const char* key = NULL;
    const char* value = NULL;
    std::map<std::string, std::string> envMap;
    while (*env) {
        if (key = NULL) {
            key = env;
        } else if (value = NULL) {
            value = env;
            envMap[key] = value;
            key = NULL;
            value = NULL;
        }
        env += strlen(env) + 1;
    }
}

inline void environmentAppend(std::vector<char> &out, const std::string &item) {
    size_t oldSize = out.size() - 1;
    out.resize(oldSize + item.size() + 1);
    memcpy(out.data() + oldSize, item.c_str(), item.size() + 1);
    out[out.size() - 1] = 0;
}

inline std::vector<char> serializeEnvironmentMap(const std::map<std::string, std::string> &envs) {
  std::vector<char> out;
  out.resize(1);
  out[0] = 0;
  for (auto it = envs.begin(); it != envs.end(); ++it) {
    environmentAppend(out, it->first);
    environmentAppend(out, it->second);
  }
  return out;
}
DaanDeMeyer commented 5 years ago

The latest commit on master adds custom environment support. Can you give it a try?

lygstate commented 5 years ago

Works very well, thanks a lot:)

DaanDeMeyer commented 5 years ago

I updated the custom environment support in reproc++ to allow passing any containers containing environment key/value pairs which reproc++ then converts to the correct format required by reproc_start. This should make using custom environments from C++ less of a hassle.