Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

wil::to_array_view provides safer access to IBuffer / IMemoryBuffer #359

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 72 additions & 0 deletions include/wil/cppwinrt_helpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,50 @@ namespace wil::details
#endif // __WIL_CPPWINRT_MICROSOFT_UI_DISPATCHING_HELPERS
/// @endcond

#if defined(WINRT_Windows_Foundation_H) && !defined(__WIL_CPPWINRT_WINDOWS_FOUNDATION_HELPERS)
#define __WIL_CPPWINRT_WINDOWS_FOUNDATION_HELPERS
namespace Windows::Foundation
dunhor marked this conversation as resolved.
Show resolved Hide resolved
{
/// @cond
struct IMemoryBufferByteAccess;
/// @endcond
}

namespace wil
{
//! Returns a view into the underlying bytes of a memory buffer
//! provided in the form of an IMemoryBufferReference.
//! The caller is responsible for ensuring that the memory buffer's
//! lifetime encompasses the lifetime of the returned view.
//! By default, returns an array_view<uint8_t>, but you can provide an alternate
//! type such as to_array_view<double>.
//! You must include memorybuffer.h in order to use this overload of to_array_view.
oldnewthing marked this conversation as resolved.
Show resolved Hide resolved
template<typename T = uint8_t>
winrt::array_view<T> to_array_view(winrt::Windows::Foundation::IMemoryBufferReference const& reference)
{
uint8_t* data;
uint32_t capacity;
// Make IMemoryBufferByteAccess a dependent type so we can talk about it even if <memorybuffer.h> hasn't been included.
using IMemoryBufferByteAccess = std::enable_if_t<std::is_same_v<T, T>, ::Windows::Foundation::IMemoryBufferByteAccess>;
winrt::check_hresult(reference.as<IMemoryBufferByteAccess>()->GetBuffer(&data, &capacity));
return { reinterpret_cast<T*>(data), static_cast<uint32_t>(capacity / sizeof(T)) };
}

//! Returns a view into the underlying bytes of a memory buffer
//! provided in the form of an IMemoryBuffer.
//! The caller is responsible for ensuring that the memory buffer's
//! lifetime encompasses the lifetime of the returned view.
//! By default, returns an array_view<uint8_t>, but you can provide an alternate
//! type such as to_array_view<double>.
//! You must include memorybuffer.h in order to use this overload of to_array_view.
template<typename T = uint8_t>
winrt::array_view<T> to_array_view(winrt::Windows::Foundation::IMemoryBuffer const& buffer)
{
return to_array_view<T>(buffer.CreateReference());
}
}
#endif

#if defined(WINRT_Windows_Foundation_Collections_H) && !defined(__WIL_CPPWINRT_WINDOWS_FOUNDATION_COLLECTION_HELPERS)
#define __WIL_CPPWINRT_WINDOWS_FOUNDATION_COLLECTION_HELPERS
namespace wil
Expand Down Expand Up @@ -314,6 +358,34 @@ namespace wil
}
#endif

#if defined(WINRT_Windows_Storage_Streams_H) && !defined(__WIL_CPPWINRT_WINDOWS_STORAGE_STREAMS_HELPERS)
#define __WIL_CPPWINRT_WINDOWS_STORAGE_STREAMS_HELPERS
namespace wil
{
//! Returns a view into the underlying bytes of an IBuffer up to its Length.
//! The caller is responsible for ensuring that the IBuffer's
//! lifetime encompasses the lifetime of the returned view.
//! By default, returns an array_view<uint8_t>, but you can provide an alternate
//! type such as to_array_view<double>.
template<typename T = uint8_t>
winrt::array_view<T> to_array_view(winrt::Windows::Storage::Streams::IBuffer const& buffer)
{
return { reinterpret_cast<T*>(buffer.data()), static_cast<uint32_t>(buffer.Length() / sizeof(T)) };
}

//! Returns a view into the underlying bytes of an IBuffer up to its Capacity.
//! The caller is responsible for ensuring that the IBuffer's
//! lifetime encompasses the lifetime of the returned view.
//! By default, returns an array_view<uint8_t>, but you can provide an alternate
//! type such as to_array_view<double>.
template<typename T = uint8_t>
winrt::array_view<T> to_array_view_for_capacity(winrt::Windows::Storage::Streams::IBuffer const& buffer)
{
return { reinterpret_cast<T*>(buffer.data()), static_cast<uint32_t>(buffer.Capacity() / sizeof(T)) };
}
}
#endif

#if defined(WINRT_Windows_UI_H) && defined(_WINDOWS_UI_INTEROP_H_) && !defined(__WIL_CPPWINRT_WINDOWS_UI_INTEROP_HELPERS)
#define __WIL_CPPWINRT_WINDOWS_UI_INTEROP_HELPERS
#if !defined(____x_ABI_CWindows_CFoundation_CIClosable_FWD_DEFINED__) && !defined(MIDL_NS_PREFIX)
Expand Down
40 changes: 40 additions & 0 deletions tests/CppWinRTTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@
#include <winrt/Windows.ApplicationModel.Activation.h>
#include <wil/cppwinrt_helpers.h>
#include <winrt/Windows.System.h>
#include <winrt/Windows.Storage.Streams.h>
#include <wil/cppwinrt_helpers.h> // Verify can include a second time to unlock more features
#include <memorybuffer.h>

using namespace winrt::Windows::ApplicationModel::Activation;

Expand Down Expand Up @@ -182,6 +184,44 @@ TEST_CASE("CppWinRTTests::VectorToVector", "[cppwinrt]")
winrt::uninit_apartment();
}

TEST_CASE("CppWinRTTests::BufferToArrayView", "[cppwinrt]")
{
std::array<int32_t, 2> testData = { 314159265, 27182818 };
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
std::array<int32_t, 2> testData = { 314159265, 27182818 };
winrt::init_apartment();
auto uninit = wil::scope_exit([]{ winrt::uninit_apartment(); });
std::array<int32_t, 2> testData = { 314159265, 27182818 };

This seems to avoid the crash. Note that I still hit the crash locally for other tests, however AFAIK we haven't hit the same in the build labs

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OMG! This comment exists for the CppWinRTAuthoringTests and I didn't even know about it!:

// This test cannot run in the same process as the malloc spies tests in wiTest.cpp
// MSFT_internal: https://task.ms/44191550

Making that [LocalOnly] isn't really the correct move, though...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dunhor Can we proceed with this PR? We sort of gave up on the NotifyPropertyChanged test (PRs 368, 374).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've disabled the NotifyPropertyChanged test to run during CI. We can either do the same thing here or we can update the test to call CoInitialize to avoid the crash during shutdown.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Err, actually disabling for just CI is probably not the best move. If we go the disable route, we should disable even when run locally so that the scripts don't break.

auto testDataByteStart = reinterpret_cast<uint8_t*>(testData.data());

// Create a buffer with capacity for our testData, length for one of the int32_t's.
auto buffer = winrt::Windows::Storage::Streams::Buffer(sizeof(testData));
buffer.Length(sizeof(int32_t));

// Get a Capacity-based int view and set the test data.
{
auto view = wil::to_array_view_for_capacity<int32_t>(buffer);
REQUIRE(view.size() == testData.size());
std::copy(view.begin(), view.end(), testData.begin());
}
// Get a Length-based byte view and confirm that the four bytes match the first four
// bytes of our test data.
{
auto view = wil::to_array_view(buffer);
REQUIRE(view.size() == sizeof(int32_t));
REQUIRE(view == winrt::array_view(testDataByteStart, sizeof(int32_t)));
}
// Create an IMemoryBuffer around the Buffer. This uses the Buffer's Capacity as the MemoryBuffer size.
auto mbuffer = winrt::Windows::Storage::Streams::Buffer::CreateMemoryBufferOverIBuffer(buffer);
// Verify that the buffer is the test data as int32_t.
{
auto view = wil::to_array_view<int32_t>(mbuffer);
REQUIRE(view.size() == testData.size());
REQUIRE(view == winrt::array_view(testData));
}
// Verify that the buffer reference gives us the test data as uint8_t.
{
auto view = wil::to_array_view(mbuffer.CreateReference());
REQUIRE(view.size() == sizeof(testData));
REQUIRE(view == winrt::array_view(testDataByteStart, sizeof(testData)));
}
}

TEST_CASE("CppWinRTTests::WilToCppWinRTExceptionTranslationTest", "[cppwinrt]")
{
auto test = [](HRESULT hr)
Expand Down
Loading