Repository: microsoft/terminal
File: src/inc/conpty-universal.h (C/C++)

Code before:

HRESULT EnvironmentMapToEnvironmentStringsW(EnvironmentVariableMapW& map, std::vector<wchar_t>& newEnvVars);

// Function Description:
// - Updates an EnvironmentVariableMapW with the current process's unicode
//   environment variables ignoring ones already set in the provided map.
// Arguments:
// - map: The map to populate with the current processes's environment variables.
// Return Value:
// - S_OK if we succeeded, or an appropriate HRESULT for failing
__declspec(noinline) inline
HRESULT UpdateEnvironmentMapW(EnvironmentVariableMapW& map)
{
    LPWCH currentEnvVars{};
    auto freeCurrentEnv = wil::scope_exit([&] {
        if (currentEnvVars)
        {
            (void)FreeEnvironmentStringsW(currentEnvVars);
            currentEnvVars = nullptr;
        }
    });

    RETURN_IF_NULL_ALLOC(currentEnvVars = ::GetEnvironmentStringsW());

    // Block is guaranteed to be double-NULL terminated at a minimum.
    for (wchar_t const* lastCh{ currentEnvVars }; *lastCh != '\0';)
    {
        // Copy current entry into temporary map.
        const size_t cchEntry{ ::wcslen(lastCh) };
        const std::wstring_view entry{ lastCh, cchEntry };

        // Every entry is of the form "name=value\0".
        auto pos = entry.find_first_of(L"=", 0, 1);
        RETURN_HR_IF(E_UNEXPECTED, pos == std::wstring::npos);

        std::wstring name{ lastCh, pos };
        std::wstring value{ lastCh + pos + 1, cchEntry - pos - 1 };

        // Don't replace entries that already exist.
        map.try_emplace(std::move(name), std::move(value));

        lastCh += cchEntry + 1; // Each entry is NULL-terminated.
    }

    return S_OK;
}

// Function Description:
// - Creates a new environment block using the provided vector as appropriate
//   (resizing if needed) based on the provided environment variable map
//   matching the format of GetEnvironmentStringsW.

Review comment (suggestion):
I don't think I've ever seen a for loop initialized like this, I think
```suggestion
    for (wchar_t const* lastCh = currentEnvVars; *lastCh != '\0';)
```

would be prefered

Code after:

[[nodiscard]]
HRESULT UpdateEnvironmentMapW(EnvironmentVariableMapW& map) noexcept;

[[nodiscard]]
HRESULT EnvironmentMapToEnvironmentStringsW(EnvironmentVariableMapW& map, std::vector<wchar_t>& newEnvVars) noexcept;

// Function Description:
// - Updates an EnvironmentVariableMapW with the current process's unicode
//   environment variables ignoring ones already set in the provided map.
// Arguments:
// - map: The map to populate with the current processes's environment variables.
// Return Value:
// - S_OK if we succeeded, or an appropriate HRESULT for failing
[[nodiscard]]
__declspec(noinline) inline
HRESULT UpdateEnvironmentMapW(EnvironmentVariableMapW& map) noexcept
{
    LPWCH currentEnvVars{};
    auto freeCurrentEnv = wil::scope_exit([&] {
        if (currentEnvVars)
        {
            (void)FreeEnvironmentStringsW(currentEnvVars);
            currentEnvVars = nullptr;
        }
    });

    RETURN_IF_NULL_ALLOC(currentEnvVars = ::GetEnvironmentStringsW());

    // Each entry is NULL-terminated; block is guaranteed to be double-NULL terminated at a minimum.
    for (wchar_t const* lastCh{ currentEnvVars }; *lastCh != '\0'; ++lastCh)
    {
        // Copy current entry into temporary map.
        const size_t cchEntry{ ::wcslen(lastCh) };
        const std::wstring_view entry{ lastCh, cchEntry };

        // Every entry is of the form "name=value\0".
        auto pos = entry.find_first_of(L"=", 0, 1);
        RETURN_HR_IF(E_UNEXPECTED, pos == std::wstring::npos);

        std::wstring name{ entry.substr(0, pos) }; // portion before '='
        std::wstring value{ entry.substr(pos + 1) }; // portion after '='

        // Don't replace entries that already exist.
        map.try_emplace(std::move(name), std::move(value));
        lastCh += cchEntry;
    }

    return S_OK;
}