Repository: microsoft/terminal
File: src/types/Environment.cpp (C++)

Code before:
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

#include "precomp.h"
#include "inc/Environment.hpp"

using namespace ::Microsoft::Console::Utils;

// We cannot use spand or not_null because we're dealing with \0\0-terminated buffers of unknown length
#pragma warning(disable : 26481 26429)

// Function Description:
// - Wraps win32's CreateEnvironmentBlock to return a smart pointer.
EnvironmentBlockPtr Microsoft::Console::Utils::CreateEnvironmentBlock()
{
    void* newEnvironmentBlock{ nullptr };
    if (!::CreateEnvironmentBlock(&newEnvironmentBlock, GetCurrentProcessToken(), FALSE))
    {
        return nullptr;
    }
    return EnvironmentBlockPtr{ newEnvironmentBlock };
}

// 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.
// - environmentBlock: Optional environment block to use when filling map. If omitted,
//   defaults to the current environment.
// Return Value:
// - S_OK if we succeeded, or an appropriate HRESULT for failing
HRESULT Microsoft::Console::Utils::UpdateEnvironmentMapW(EnvironmentVariableMapW& map, std::optional<void*> environmentBlock) noexcept
try
{
    wchar_t const* activeEnvironmentBlock{ reinterpret_cast<wchar_t const*>(environmentBlock.value_or(nullptr)) };

    wil::unique_environstrings_ptr currentEnvVars;
    if (!activeEnvironmentBlock)
    {
        currentEnvVars.reset(::GetEnvironmentStringsW());
        RETURN_HR_IF_NULL(E_OUTOFMEMORY, currentEnvVars);
        activeEnvironmentBlock = currentEnvVars.get();
    }

Review comment (bug):
It seems a bit strange to take `std::optional<void*>` and then treat `nullptr` as equivalent to `std::nullopt`. I don't see `std::optional<T*>` anywhere else in the source tree. But perhaps it makes sense if you consider `nullptr` an invalid argument here.

Code after:
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

#include "precomp.h"
#include "inc/Environment.hpp"

using namespace ::Microsoft::Console::Utils;

// We cannot use spand or not_null because we're dealing with \0\0-terminated buffers of unknown length
#pragma warning(disable : 26481 26429)

// Function Description:
// - Wraps win32's CreateEnvironmentBlock to return a smart pointer.
EnvironmentBlockPtr Microsoft::Console::Utils::CreateEnvironmentBlock()
{
    void* newEnvironmentBlock{ nullptr };
    if (!::CreateEnvironmentBlock(&newEnvironmentBlock, GetCurrentProcessToken(), FALSE))
    {
        return nullptr;
    }
    return EnvironmentBlockPtr{ newEnvironmentBlock };
}

// 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.
// - environmentBlock: Optional environment block to use when filling map. If omitted,
//   defaults to the current environment.
// Return Value:
// - S_OK if we succeeded, or an appropriate HRESULT for failing
HRESULT Microsoft::Console::Utils::UpdateEnvironmentMapW(EnvironmentVariableMapW& map, void* environmentBlock) noexcept
try
{
    wchar_t const* activeEnvironmentBlock{ static_cast<wchar_t const*>(environmentBlock) };

    wil::unique_environstrings_ptr currentEnvVars;
    if (!activeEnvironmentBlock)
    {
        currentEnvVars.reset(::GetEnvironmentStringsW());
        RETURN_HR_IF_NULL(E_OUTOFMEMORY, currentEnvVars);
        activeEnvironmentBlock = currentEnvVars.get();
    }