I want to expand a string ("%LOCALAPPDATA%/test.txt") with a Windows environment path. The following function in principle does the job, but calling it again with the same output string (or assigning some value to the output string before calling the function) gives a segfault.

Obviously I am making some (probably really bad) mistake by converting char* to std::string, but I don't really understand what is going on here (beside the fact that some memory address is not available later on).

#include "processenv.h"

void expandWindowsString(const std::string &input, std::string &output)
{
    const char* in = input.c_str();
    char* out;
    ExpandEnvironmentStrings(in, out, 1024);
    output = out;
}

int main(int argc, char *argv[])
{
    std::string path;
    expandWindowsString("%LOCALAPPDATA%/test.txt", path);
    std::cout << "path is " << path << std::endl;
    //works fine so far, but if I execute the function again (with 'path') or initialising path beforehand with std::string path = "", a segfault occurs.
    expandWindowsString("%LOCALAPPDATA%/test.txt", path); // commenting out this line, makes the code work.
    std::cout << "path is " << path << "\n";
}

CodePudding user response：

As the documentation explains:

lpDst

A pointer to a buffer that receives the result of expanding the environment variable strings in the lpSrc buffer.

The buffer needs to be supplied by the caller, while the code simply passes an uninitialized pointer, alongside tricking the system into believing that it points at memory with a size of 1024 bytes.

A simple fix would be:

void expandWindowsString(const std::string &input, std::string &output)
{
    const char* in = input.c_str();
    char out[1024];
    ExpandEnvironmentStrings(in, out, 1024);
    output = out;
}

There's lots of room for improvement, e.g.

• Using the Unicode version
• Handling errors (as explained in the documentation)
• Repeatedly growing the output buffer in case the API call returns a value larger than the provided buffer length
• Constructing a std::string using the pointer and length
• Returning a value rather than using an out parameter

As for why it fails when being called a second time: That's a meaningless question. The code exhibits undefined behavior by writing through an uninitialized pointer. With that, any outcome is possible, including the code appearing to work as expected.

CodePudding user response：

If you read the doc

lpDst

A pointer to a buffer that receives the result of expanding the environment variable strings in the lpSrc buffer. Note that this buffer cannot be the same as the lpSrc buffer.

nSize

The maximum number of characters that can be stored in the buffer pointed to by the lpDst parameter. When using ANSI strings, the buffer size should be the string length, plus terminating null character, plus one. When using Unicode strings, the buffer size should be the string length plus the terminating null character.

You will see you pass invalid parameter, you have to provide buffer, so:

void expandWindowsString(const std::string &input, std::string &output)
{
    char out[1024];
    ExpandEnvironmentStrings(input.c_str(), out, sizeof (out));
    output = out;
}

or avoiding output parameter:

std::string expandWindowsString(const std::string &input)
{
    char out[1024];
    ExpandEnvironmentStrings(input.c_str(), out, sizeof (out));
    return out;
}

Notice that you should probably check return value of ExpandEnvironmentStrings.

CodePudding user response：

ExpandEnvironmentStrings expects a buffer already allocated. You can determine the size of the buffer if you pass a buffer with a size too small to hold the string, like 0.

So, here are two versions, UNICODE and ANSI, that will compute the size dynamically:

std::wstring expandWindowsString(const std::wstring& input)
{
    auto size = ExpandEnvironmentStringsW(input.c_str(), nullptr, 0);
    std::wstring output;
    if (size)
    {
        output.reserve(size);
        ExpandEnvironmentStringsW(input.c_str(), (LPWSTR)output.c_str(), size);
    }
    return output;
}

std::string expandWindowsString(const std::string& input)
{
    auto size = ExpandEnvironmentStringsA(input.c_str(), nullptr, 0);
    std::string output;
    if (size)
    {
        output.reserve(size);
        ExpandEnvironmentStringsA(input.c_str(), (LPSTR)output.c_str(), size);
    }
    return output;
}