Code review for invalid args validation (#585)

Author: walbourn

Audio/DynamicSoundEffectInstance.cpp

@@ -31,6 +31,9 @@ class DynamicSoundEffectInstance::Impl : public IVoiceNotify
         mBufferNeeded(nullptr),
         mObject(object)
     {
+        if (!engine)
+            throw std::invalid_argument("AudioEngine is required");
+
         if ((sampleRate < XAUDIO2_MIN_SAMPLE_RATE)
             || (sampleRate > XAUDIO2_MAX_SAMPLE_RATE))
         {
@@ -55,6 +58,12 @@ class DynamicSoundEffectInstance::Impl : public IVoiceNotify
             throw std::invalid_argument("DynamicSoundEffectInstance supports 8 or 16 bit");
         }
 
+        if (!bufferNeeded)
+        {
+            DebugTrace("DynamicSoundEffectInstance requires a valid callback\n");
+            throw std::invalid_argument("DynamicSoundEffectInstance");
+        }
+
         mBufferEvent.reset(CreateEventEx(nullptr, nullptr, 0, EVENT_MODIFY_STATE | SYNCHRONIZE));
         if (!mBufferEvent)
         {
@@ -63,7 +72,6 @@ class DynamicSoundEffectInstance::Impl : public IVoiceNotify
 
         CreateIntegerPCM(&mWaveFormat, sampleRate, channels, sampleBits);
 
-        assert(engine != nullptr);
         engine->RegisterNotify(this, true);
 
         mBase.Initialize(engine, &mWaveFormat, flags);

Audio/SoundCommon.cpp

@@ -521,6 +521,9 @@ void DirectX::CreateIntegerPCM(
     int channels,
     int sampleBits) noexcept
 {
+    if (!wfx)
+        return;
+
     const int blockAlign = channels * sampleBits / 8;
 
     wfx->wFormatTag = WAVE_FORMAT_PCM;
@@ -541,6 +544,9 @@ void DirectX::CreateFloatPCM(
     int sampleRate,
     int channels) noexcept
 {
+    if (!wfx)
+        return;
+
     const int blockAlign = channels * 4;
 
     wfx->wFormatTag = WAVE_FORMAT_IEEE_FLOAT;
@@ -563,6 +569,9 @@ void DirectX::CreateADPCM(
     int channels,
     int samplesPerBlock) noexcept(false)
 {
+    if (!wfx)
+        return;
+
     if (wfxSize < (sizeof(WAVEFORMATEX) + MSADPCM_FORMAT_EXTRA_BYTES))
     {
         DebugTrace("CreateADPCM needs at least %zu bytes for the result\n",
@@ -608,6 +617,9 @@ void DirectX::CreateXWMA(
     int avgBytes,
     bool wma3) noexcept
 {
+    if (!wfx)
+        return;
+
     wfx->wFormatTag = static_cast<WORD>((wma3) ? WAVE_FORMAT_WMAUDIO3 : WAVE_FORMAT_WMAUDIO2);
     wfx->nChannels = static_cast<WORD>(channels);
     wfx->nSamplesPerSec = static_cast<DWORD>(sampleRate);

Audio/SoundCommon.h

@@ -13,6 +13,8 @@
 #include "Audio.h"
 #include "PlatformHelpers.h"
 
+#include <stdexcept>
+
 #ifdef USING_XAUDIO2_9
 #define DIRECTX_ENABLE_XWMA
 #endif

Audio/SoundEffect.cpp

@@ -50,7 +50,9 @@ class SoundEffect::Impl : public IVoiceNotify
         , mXMAMemory(nullptr)
     #endif
     {
-        assert(mEngine != nullptr);
+        if (!engine)
+            throw std::invalid_argument("AudioEngine is required");
+
         mEngine->RegisterNotify(this, false);
     }
 

Audio/WaveBank.cpp

@@ -33,7 +33,9 @@ class WaveBank::Impl : public IVoiceNotify
         mPrepared(false),
         mStreaming(false)
     {
-        assert(mEngine != nullptr);
+        if (!engine)
+            throw std::invalid_argument("AudioEngine is required");
+
         mEngine->RegisterNotify(this, false);
     }
 

Audio/WaveBankReader.cpp

@@ -517,6 +517,9 @@ HRESULT WaveBankReader::Impl::Open(const wchar_t* szFileName) noexcept(false)
     Close();
     Clear();
 
+    if (!szFileName)
+        return E_INVALIDARG;
+
     m_prepared = false;
 
     m_event.reset(CreateEventEx(nullptr, nullptr, CREATE_EVENT_MANUAL_RESET, EVENT_MODIFY_STATE | SYNCHRONIZE));
@@ -1029,7 +1032,7 @@ HRESULT WaveBankReader::Impl::GetWaveData(uint32_t index, const uint8_t** pData,
 #endif
 
     if (!waveData)
-        return E_FAIL;
+        return E_POINTER;
 
     if (m_data.dwFlags & BANKDATA::TYPE_STREAMING)
     {
@@ -1223,10 +1226,13 @@ HRESULT WaveBankReader::Open(const wchar_t* szFileName) noexcept
 _Use_decl_annotations_
 uint32_t WaveBankReader::Find(const char* name) const
 {
-    auto it = pImpl->m_names.find(name);
-    if (it != pImpl->m_names.cend())
+    if (name)
     {
-        return it->second;
+        auto it = pImpl->m_names.find(name);
+        if (it != pImpl->m_names.cend())
+        {
+            return it->second;
+        }
     }
 
     return uint32_t(-1);

README.md

@@ -88,6 +88,8 @@ For a full change history, see [CHANGELOG.md](https://github.com/microsoft/Direc
 
   * If you want to use XAudio2Redist with Windows 8.1, the CMake project supports this with the build option `BUILD_XAUDIO_REDIST`. The CMake build option `BUILD_XAUDIO_WIN7` was renamed.
 
+  * When using XAudio 2.8 for Windows 8.1, there is no xWMA format support. This is available when using XAudio 2.9 or XAudio2Redist.
+
 * Starting with the February 2023 release, the Mouse class implementation of relative mouse movement was updated to accumulate changes between calls to ``GetState``. By default, each time you call ``GetState`` the deltas are reset which works for scenarios where you use relative movement but only call the method once per frame. If you call it more than once per frame, then add an explicit call to ``EndOfInputFrame`` to use an explicit reset model instead.
 
 * As of the September 2022 release, the library makes use of C++11 inline namespaces for differing types that have the same names in the DirectX 11 and DirectX 12 version of the _DirectX Tool Kit_. This provides a link-unique name such as ``DirectX::DX11::SpriteBatch`` that will appear in linker output messages. In most use cases, however, there is no need to add explicit ``DX11`` namespace resolution in client code.
@@ -116,7 +118,7 @@ For a full change history, see [CHANGELOG.md](https://github.com/microsoft/Direc
 
 ## Support
 
-For questions, consider using [Stack Overflow](https://stackoverflow.com/questions/tagged/directxtk) with the _directxtk_ tag, or the [DirectX Discord Server](https://discord.gg/directx) in the _dx9-dx11-developers_ channel.
+For questions, consider using [Stack Overflow](https://stackoverflow.com/questions/tagged/directxtk) with the _directxtk_ tag, or the [DirectX Discord Server](https://discord.gg/directx) in the _dx9-dx11-developers_ or _input-and-audio_ channels.
 
 For bug reports and feature requests, please use GitHub [issues](https://github.com/microsoft/DirectXTK/issues) for this project.
 

Src/BinaryReader.h

@@ -27,6 +27,9 @@ namespace DirectX
         explicit BinaryReader(_In_z_ wchar_t const* fileName) noexcept(false);
         BinaryReader(_In_reads_bytes_(dataSize) uint8_t const* dataBlob, size_t dataSize) noexcept;
 
+        BinaryReader(BinaryReader&&) noexcept;
+        BinaryReader& operator= (BinaryReader&&) noexcept;
+
         BinaryReader(BinaryReader const&) = delete;
         BinaryReader& operator= (BinaryReader const&) = delete;
 

Src/DGSLEffectFactory.cpp

@@ -34,7 +34,10 @@ class DGSLEffectFactory::Impl
         mDevice(device),
         mSharing(true),
         mForceSRGB(false)
-    {}
+    {
+        if (!device)
+            throw std::invalid_argument("Direct3D device is null");
+    }
 
     Impl(const Impl&) = delete;
     Impl& operator=(const Impl&) = delete;

Src/GraphicsMemory.cpp

@@ -64,7 +64,9 @@ class GraphicsMemory::Impl
 
     void Initialize(_In_ ID3D11DeviceX* device, unsigned int backBufferCount)
     {
-        assert(device != nullptr);
+        if (!device)
+            throw std::invalid_argument("Direct3D device is null");
+
         mDevice = device;
 
         device->GetImmediateContextX(mDeviceContext.GetAddressOf());
@@ -245,9 +247,11 @@ class GraphicsMemory::Impl
         s_graphicsMemory = nullptr;
     }
 
-    void Initialize(_In_ ID3D11Device* device, unsigned int backBufferCount) noexcept
+    void Initialize(_In_ ID3D11Device* device, unsigned int backBufferCount)
     {
-        UNREFERENCED_PARAMETER(device);
+        if (!device)
+            throw std::invalid_argument("Direct3D device is null");
+
         UNREFERENCED_PARAMETER(backBufferCount);
     }