From b6a68e8d510610e181d638ff993e327059bd6018 Mon Sep 17 00:00:00 2001 From: Chris Robinson Date: Sun, 3 Dec 2023 23:36:07 -0800 Subject: Remove some unnecessary atomic wrappers --- al/buffer.cpp | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) (limited to 'al/buffer.cpp') diff --git a/al/buffer.cpp b/al/buffer.cpp index 7d043036..ae41585f 100644 --- a/al/buffer.cpp +++ b/al/buffer.cpp @@ -286,7 +286,7 @@ void LoadData(ALCcontext *context, ALbuffer *ALBuf, ALsizei freq, ALuint size, const FmtChannels DstChannels, const FmtType DstType, const std::byte *SrcData, ALbitfieldSOFT access) { - if(ReadRef(ALBuf->ref) != 0 || ALBuf->MappedAccess != 0) UNLIKELY + if(ALBuf->ref.load(std::memory_order_relaxed) != 0 || ALBuf->MappedAccess != 0) UNLIKELY return context->setError(AL_INVALID_OPERATION, "Modifying storage for in-use buffer %u", ALBuf->id); @@ -393,7 +393,7 @@ void PrepareCallback(ALCcontext *context, ALbuffer *ALBuf, ALsizei freq, const FmtChannels DstChannels, const FmtType DstType, ALBUFFERCALLBACKTYPESOFT callback, void *userptr) { - if(ReadRef(ALBuf->ref) != 0 || ALBuf->MappedAccess != 0) UNLIKELY + if(ALBuf->ref.load(std::memory_order_relaxed) != 0 || ALBuf->MappedAccess != 0) UNLIKELY return context->setError(AL_INVALID_OPERATION, "Modifying callback for in-use buffer %u", ALBuf->id); @@ -445,7 +445,7 @@ void PrepareCallback(ALCcontext *context, ALbuffer *ALBuf, ALsizei freq, void PrepareUserPtr(ALCcontext *context, ALbuffer *ALBuf, ALsizei freq, const FmtChannels DstChannels, const FmtType DstType, std::byte *sdata, const ALuint sdatalen) { - if(ReadRef(ALBuf->ref) != 0 || ALBuf->MappedAccess != 0) UNLIKELY + if(ALBuf->ref.load(std::memory_order_relaxed) != 0 || ALBuf->MappedAccess != 0) UNLIKELY return context->setError(AL_INVALID_OPERATION, "Modifying storage for in-use buffer %u", ALBuf->id); @@ -711,7 +711,7 @@ FORCE_ALIGN void AL_APIENTRY alDeleteBuffersDirect(ALCcontext *context, ALsizei context->setError(AL_INVALID_NAME, "Invalid buffer ID %u", bid); return false; } - if(ReadRef(ALBuf->ref) != 0) UNLIKELY + if(ALBuf->ref.load(std::memory_order_relaxed) != 0) UNLIKELY { context->setError(AL_INVALID_OPERATION, "Deleting in-use buffer %u", bid); return false; @@ -826,7 +826,8 @@ FORCE_ALIGN void* AL_APIENTRY alMapBufferDirectSOFT(ALCcontext *context, ALuint else { ALbitfieldSOFT unavailable = (albuf->Access^access) & access; - if(ReadRef(albuf->ref) != 0 && !(access&AL_MAP_PERSISTENT_BIT_SOFT)) UNLIKELY + if(albuf->ref.load(std::memory_order_relaxed) != 0 + && !(access&AL_MAP_PERSISTENT_BIT_SOFT)) UNLIKELY context->setError(AL_INVALID_OPERATION, "Mapping in-use buffer %u without persistent mapping", buffer); else if(albuf->MappedAccess != 0) UNLIKELY @@ -1042,7 +1043,7 @@ FORCE_ALIGN void AL_APIENTRY alBufferiDirect(ALCcontext *context, ALuint buffer, break; case AL_AMBISONIC_LAYOUT_SOFT: - if(ReadRef(albuf->ref) != 0) UNLIKELY + if(albuf->ref.load(std::memory_order_relaxed) != 0) UNLIKELY context->setError(AL_INVALID_OPERATION, "Modifying in-use buffer %u's ambisonic layout", buffer); else if(const auto layout = AmbiLayoutFromEnum(value)) @@ -1052,7 +1053,7 @@ FORCE_ALIGN void AL_APIENTRY alBufferiDirect(ALCcontext *context, ALuint buffer, break; case AL_AMBISONIC_SCALING_SOFT: - if(ReadRef(albuf->ref) != 0) UNLIKELY + if(albuf->ref.load(std::memory_order_relaxed) != 0) UNLIKELY context->setError(AL_INVALID_OPERATION, "Modifying in-use buffer %u's ambisonic scaling", buffer); else if(const auto scaling = AmbiScalingFromEnum(value)) @@ -1116,7 +1117,7 @@ FORCE_ALIGN void AL_APIENTRY alBufferivDirect(ALCcontext *context, ALuint buffer else switch(param) { case AL_LOOP_POINTS_SOFT: - if(ReadRef(albuf->ref) != 0) UNLIKELY + if(albuf->ref.load(std::memory_order_relaxed) != 0) UNLIKELY context->setError(AL_INVALID_OPERATION, "Modifying in-use buffer %u's loop points", buffer); else if(values[0] < 0 || values[0] >= values[1] -- cgit v1.2.3 From 305cbdf7425e58226953fb73c2cff809a74de9a1 Mon Sep 17 00:00:00 2001 From: Chris Robinson Date: Mon, 4 Dec 2023 21:12:59 -0800 Subject: Check for a valid alignment with callback buffers --- al/buffer.cpp | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'al/buffer.cpp') diff --git a/al/buffer.cpp b/al/buffer.cpp index ae41585f..e577e17a 100644 --- a/al/buffer.cpp +++ b/al/buffer.cpp @@ -402,6 +402,10 @@ void PrepareCallback(ALCcontext *context, ALbuffer *ALBuf, ALsizei freq, const ALuint unpackalign{ALBuf->UnpackAlign}; const ALuint align{SanitizeAlignment(DstType, unpackalign)}; + if(align < 1) UNLIKELY + return context->setError(AL_INVALID_VALUE, "Invalid unpack alignment %u for %s samples", + unpackalign, NameFromFormat(DstType)); + const ALuint BlockSize{ChannelsFromFmt(DstChannels, ambiorder) * ((DstType == FmtIMA4) ? (align-1)/2 + 4 : (DstType == FmtMSADPCM) ? (align-2)/2 + 7 : -- cgit v1.2.3 From 205a73876234c0b1363189306530ada73ece56f2 Mon Sep 17 00:00:00 2001 From: Chris Robinson Date: Tue, 26 Dec 2023 22:37:12 -0800 Subject: Try to start being a bit more pointer-owner conscious --- al/auxeffectslot.cpp | 25 +++++++---------- al/buffer.cpp | 3 +- al/effect.cpp | 3 +- al/filter.cpp | 3 +- alc/alc.cpp | 2 +- alc/backends/wave.cpp | 78 +++++++++++++++++++++++++-------------------------- alc/context.h | 4 +-- alc/device.h | 6 ++-- common/almalloc.cpp | 12 ++++---- common/almalloc.h | 59 +++++++++++++++++++++++++------------- common/flexarray.h | 4 +-- core/context.cpp | 24 +++++----------- 12 files changed, 115 insertions(+), 108 deletions(-) (limited to 'al/buffer.cpp') diff --git a/al/auxeffectslot.cpp b/al/auxeffectslot.cpp index 408b742b..ea41a842 100644 --- a/al/auxeffectslot.cpp +++ b/al/auxeffectslot.cpp @@ -155,19 +155,17 @@ void AddActiveEffectSlots(const al::span auxslots, ALCcontext *co */ if(newcount < newarray->size()) UNLIKELY { - curarray = newarray; + std::unique_ptr oldarray{newarray}; newarray = EffectSlot::CreatePtrArray(newcount); - std::copy_n(curarray->begin(), newcount, newarray->begin()); - delete curarray; - curarray = nullptr; + std::copy_n(oldarray->begin(), newcount, newarray->begin()); } std::uninitialized_fill_n(newarray->end(), newcount, nullptr); - curarray = context->mActiveAuxSlots.exchange(newarray, std::memory_order_acq_rel); + std::unique_ptr oldarray{context->mActiveAuxSlots.exchange(newarray, + std::memory_order_acq_rel)}; std::ignore = context->mDevice->waitForMix(); - std::destroy_n(curarray->end(), curarray->size()); - delete curarray; + std::destroy_n(oldarray->end(), oldarray->size()); } void RemoveActiveEffectSlots(const al::span auxslots, ALCcontext *context) @@ -193,20 +191,17 @@ void RemoveActiveEffectSlots(const al::span auxslots, ALCcontext auto newsize = static_cast(std::distance(newarray->begin(), new_end)); if(newsize != newarray->size()) LIKELY { - curarray = newarray; + std::unique_ptr oldarray{newarray}; newarray = EffectSlot::CreatePtrArray(newsize); - std::copy_n(curarray->begin(), newsize, newarray->begin()); - - delete curarray; - curarray = nullptr; + std::copy_n(oldarray->begin(), newsize, newarray->begin()); } std::uninitialized_fill_n(newarray->end(), newsize, nullptr); - curarray = context->mActiveAuxSlots.exchange(newarray, std::memory_order_acq_rel); + std::unique_ptr oldarray{context->mActiveAuxSlots.exchange(newarray, + std::memory_order_acq_rel)}; std::ignore = context->mDevice->waitForMix(); std::destroy_n(curarray->end(), curarray->size()); - delete curarray; } @@ -251,7 +246,7 @@ bool EnsureEffectSlots(ALCcontext *context, size_t needed) context->mEffectSlotList.emplace_back(); auto sublist = context->mEffectSlotList.end() - 1; sublist->FreeMask = ~0_u64; - sublist->EffectSlots = static_cast( + sublist->EffectSlots = static_cast>( al_calloc(alignof(ALeffectslot), sizeof(ALeffectslot)*64)); if(!sublist->EffectSlots) UNLIKELY { diff --git a/al/buffer.cpp b/al/buffer.cpp index e577e17a..c0f3f348 100644 --- a/al/buffer.cpp +++ b/al/buffer.cpp @@ -186,7 +186,8 @@ bool EnsureBuffers(ALCdevice *device, size_t needed) device->BufferList.emplace_back(); auto sublist = device->BufferList.end() - 1; sublist->FreeMask = ~0_u64; - sublist->Buffers = static_cast(al_calloc(alignof(ALbuffer), sizeof(ALbuffer)*64)); + sublist->Buffers = static_cast>(al_calloc(alignof(ALbuffer), + sizeof(ALbuffer)*64)); if(!sublist->Buffers) UNLIKELY { device->BufferList.pop_back(); diff --git a/al/effect.cpp b/al/effect.cpp index c33faa2c..c2a2d1b1 100644 --- a/al/effect.cpp +++ b/al/effect.cpp @@ -134,7 +134,8 @@ bool EnsureEffects(ALCdevice *device, size_t needed) device->EffectList.emplace_back(); auto sublist = device->EffectList.end() - 1; sublist->FreeMask = ~0_u64; - sublist->Effects = static_cast(al_calloc(alignof(ALeffect), sizeof(ALeffect)*64)); + sublist->Effects = static_cast>(al_calloc(alignof(ALeffect), + sizeof(ALeffect)*64)); if(!sublist->Effects) UNLIKELY { device->EffectList.pop_back(); diff --git a/al/filter.cpp b/al/filter.cpp index 9c8e4c62..ce37b0aa 100644 --- a/al/filter.cpp +++ b/al/filter.cpp @@ -129,7 +129,8 @@ bool EnsureFilters(ALCdevice *device, size_t needed) device->FilterList.emplace_back(); auto sublist = device->FilterList.end() - 1; sublist->FreeMask = ~0_u64; - sublist->Filters = static_cast(al_calloc(alignof(ALfilter), sizeof(ALfilter)*64)); + sublist->Filters = static_cast>(al_calloc(alignof(ALfilter), + sizeof(ALfilter)*64)); if(!sublist->Filters) UNLIKELY { device->FilterList.pop_back(); diff --git a/alc/alc.cpp b/alc/alc.cpp index e9d3aed7..64b77080 100644 --- a/alc/alc.cpp +++ b/alc/alc.cpp @@ -2735,7 +2735,7 @@ ALC_API ALCcontext* ALC_APIENTRY alcCreateContext(ALCdevice *device, const ALCin if(oldarray != &DeviceBase::sEmptyContextArray) { std::ignore = dev->waitForMix(); - delete oldarray; + newarray.reset(oldarray); } } statelock.unlock(); diff --git a/alc/backends/wave.cpp b/alc/backends/wave.cpp index 95064906..60cebd7f 100644 --- a/alc/backends/wave.cpp +++ b/alc/backends/wave.cpp @@ -55,6 +55,11 @@ using std::chrono::nanoseconds; using ubyte = unsigned char; using ushort = unsigned short; +struct FileDeleter { + void operator()(FILE *f) { fclose(f); } +}; +using FilePtr = std::unique_ptr; + /* NOLINTNEXTLINE(*-avoid-c-arrays) */ constexpr char waveDevice[] = "Wave File Writer"; @@ -102,7 +107,7 @@ struct WaveBackend final : public BackendBase { void start() override; void stop() override; - FILE *mFile{nullptr}; + FilePtr mFile{nullptr}; long mDataStart{-1}; std::vector mBuffer; @@ -111,12 +116,7 @@ struct WaveBackend final : public BackendBase { std::thread mThread; }; -WaveBackend::~WaveBackend() -{ - if(mFile) - fclose(mFile); - mFile = nullptr; -} +WaveBackend::~WaveBackend() = default; int WaveBackend::mixerProc() { @@ -168,8 +168,8 @@ int WaveBackend::mixerProc() } } - const size_t fs{fwrite(mBuffer.data(), frameSize, mDevice->UpdateSize, mFile)}; - if(fs < mDevice->UpdateSize || ferror(mFile)) + const size_t fs{fwrite(mBuffer.data(), frameSize, mDevice->UpdateSize, mFile.get())}; + if(fs < mDevice->UpdateSize || ferror(mFile.get())) { ERR("Error writing to file\n"); mDevice->handleDisconnect("Failed to write playback samples"); @@ -211,10 +211,10 @@ void WaveBackend::open(std::string_view name) #ifdef _WIN32 { std::wstring wname{utf8_to_wstr(fname.value())}; - mFile = _wfopen(wname.c_str(), L"wb"); + mFile.reset(_wfopen(wname.c_str(), L"wb")); } #else - mFile = fopen(fname->c_str(), "wb"); + mFile.reset(fopen(fname->c_str(), "wb")); #endif if(!mFile) throw al::backend_exception{al::backend_error::DeviceError, "Could not open file '%s': %s", @@ -228,8 +228,8 @@ bool WaveBackend::reset() uint channels{0}, bytes{0}, chanmask{0}; bool isbformat{false}; - fseek(mFile, 0, SEEK_SET); - clearerr(mFile); + fseek(mFile.get(), 0, SEEK_SET); + clearerr(mFile.get()); if(GetConfigValueBool({}, "wave", "bformat", false)) { @@ -280,48 +280,48 @@ bool WaveBackend::reset() bytes = mDevice->bytesFromFmt(); channels = mDevice->channelsFromFmt(); - rewind(mFile); + rewind(mFile.get()); - fputs("RIFF", mFile); - fwrite32le(0xFFFFFFFF, mFile); // 'RIFF' header len; filled in at close + fputs("RIFF", mFile.get()); + fwrite32le(0xFFFFFFFF, mFile.get()); // 'RIFF' header len; filled in at close - fputs("WAVE", mFile); + fputs("WAVE", mFile.get()); - fputs("fmt ", mFile); - fwrite32le(40, mFile); // 'fmt ' header len; 40 bytes for EXTENSIBLE + fputs("fmt ", mFile.get()); + fwrite32le(40, mFile.get()); // 'fmt ' header len; 40 bytes for EXTENSIBLE // 16-bit val, format type id (extensible: 0xFFFE) - fwrite16le(0xFFFE, mFile); + fwrite16le(0xFFFE, mFile.get()); // 16-bit val, channel count - fwrite16le(static_cast(channels), mFile); + fwrite16le(static_cast(channels), mFile.get()); // 32-bit val, frequency - fwrite32le(mDevice->Frequency, mFile); + fwrite32le(mDevice->Frequency, mFile.get()); // 32-bit val, bytes per second - fwrite32le(mDevice->Frequency * channels * bytes, mFile); + fwrite32le(mDevice->Frequency * channels * bytes, mFile.get()); // 16-bit val, frame size - fwrite16le(static_cast(channels * bytes), mFile); + fwrite16le(static_cast(channels * bytes), mFile.get()); // 16-bit val, bits per sample - fwrite16le(static_cast(bytes * 8), mFile); + fwrite16le(static_cast(bytes * 8), mFile.get()); // 16-bit val, extra byte count - fwrite16le(22, mFile); + fwrite16le(22, mFile.get()); // 16-bit val, valid bits per sample - fwrite16le(static_cast(bytes * 8), mFile); + fwrite16le(static_cast(bytes * 8), mFile.get()); // 32-bit val, channel mask - fwrite32le(chanmask, mFile); + fwrite32le(chanmask, mFile.get()); // 16 byte GUID, sub-type format std::ignore = fwrite((mDevice->FmtType == DevFmtFloat) ? (isbformat ? SUBTYPE_BFORMAT_FLOAT.data() : SUBTYPE_FLOAT.data()) : - (isbformat ? SUBTYPE_BFORMAT_PCM.data() : SUBTYPE_PCM.data()), 1, 16, mFile); + (isbformat ? SUBTYPE_BFORMAT_PCM.data() : SUBTYPE_PCM.data()), 1, 16, mFile.get()); - fputs("data", mFile); - fwrite32le(0xFFFFFFFF, mFile); // 'data' header len; filled in at close + fputs("data", mFile.get()); + fwrite32le(0xFFFFFFFF, mFile.get()); // 'data' header len; filled in at close - if(ferror(mFile)) + if(ferror(mFile.get())) { ERR("Error writing header: %s\n", strerror(errno)); return false; } - mDataStart = ftell(mFile); + mDataStart = ftell(mFile.get()); setDefaultWFXChannelOrder(); @@ -333,7 +333,7 @@ bool WaveBackend::reset() void WaveBackend::start() { - if(mDataStart > 0 && fseek(mFile, 0, SEEK_END) != 0) + if(mDataStart > 0 && fseek(mFile.get(), 0, SEEK_END) != 0) WARN("Failed to seek on output file\n"); try { mKillNow.store(false, std::memory_order_release); @@ -353,14 +353,14 @@ void WaveBackend::stop() if(mDataStart > 0) { - long size{ftell(mFile)}; + long size{ftell(mFile.get())}; if(size > 0) { long dataLen{size - mDataStart}; - if(fseek(mFile, 4, SEEK_SET) == 0) - fwrite32le(static_cast(size-8), mFile); // 'WAVE' header len - if(fseek(mFile, mDataStart-4, SEEK_SET) == 0) - fwrite32le(static_cast(dataLen), mFile); // 'data' header len + if(fseek(mFile.get(), 4, SEEK_SET) == 0) + fwrite32le(static_cast(size-8), mFile.get()); // 'WAVE' header len + if(fseek(mFile.get(), mDataStart-4, SEEK_SET) == 0) + fwrite32le(static_cast(dataLen), mFile.get()); // 'data' header len } } } diff --git a/alc/context.h b/alc/context.h index 9f49ceac..d5e5e78b 100644 --- a/alc/context.h +++ b/alc/context.h @@ -70,7 +70,7 @@ struct DebugLogEntry { struct SourceSubList { uint64_t FreeMask{~0_u64}; - ALsource *Sources{nullptr}; /* 64 */ + gsl::owner Sources{nullptr}; /* 64 */ SourceSubList() noexcept = default; SourceSubList(const SourceSubList&) = delete; @@ -85,7 +85,7 @@ struct SourceSubList { struct EffectSlotSubList { uint64_t FreeMask{~0_u64}; - ALeffectslot *EffectSlots{nullptr}; /* 64 */ + gsl::owner EffectSlots{nullptr}; /* 64 */ EffectSlotSubList() noexcept = default; EffectSlotSubList(const EffectSlotSubList&) = delete; diff --git a/alc/device.h b/alc/device.h index 4eb693ff..e5e9b3d2 100644 --- a/alc/device.h +++ b/alc/device.h @@ -35,7 +35,7 @@ using uint = unsigned int; struct BufferSubList { uint64_t FreeMask{~0_u64}; - ALbuffer *Buffers{nullptr}; /* 64 */ + gsl::owner Buffers{nullptr}; /* 64 */ BufferSubList() noexcept = default; BufferSubList(const BufferSubList&) = delete; @@ -50,7 +50,7 @@ struct BufferSubList { struct EffectSubList { uint64_t FreeMask{~0_u64}; - ALeffect *Effects{nullptr}; /* 64 */ + gsl::owner Effects{nullptr}; /* 64 */ EffectSubList() noexcept = default; EffectSubList(const EffectSubList&) = delete; @@ -65,7 +65,7 @@ struct EffectSubList { struct FilterSubList { uint64_t FreeMask{~0_u64}; - ALfilter *Filters{nullptr}; /* 64 */ + gsl::owner Filters{nullptr}; /* 64 */ FilterSubList() noexcept = default; FilterSubList(const FilterSubList&) = delete; diff --git a/common/almalloc.cpp b/common/almalloc.cpp index ad1dc6be..71953d8b 100644 --- a/common/almalloc.cpp +++ b/common/almalloc.cpp @@ -13,13 +13,13 @@ #endif -void *al_malloc(size_t alignment, size_t size) +gsl::owner al_malloc(size_t alignment, size_t size) { assert((alignment & (alignment-1)) == 0); alignment = std::max(alignment, alignof(std::max_align_t)); #if defined(HAVE_POSIX_MEMALIGN) - void *ret{}; + gsl::owner ret{}; if(posix_memalign(&ret, alignment, size) == 0) return ret; return nullptr; @@ -41,14 +41,14 @@ void *al_malloc(size_t alignment, size_t size) #endif } -void *al_calloc(size_t alignment, size_t size) +gsl::owner al_calloc(size_t alignment, size_t size) { - void *ret{al_malloc(alignment, size)}; + gsl::owner ret{al_malloc(alignment, size)}; if(ret) std::memset(ret, 0, size); return ret; } -void al_free(void *ptr) noexcept +void al_free(gsl::owner ptr) noexcept { #if defined(HAVE_POSIX_MEMALIGN) std::free(ptr); @@ -56,6 +56,6 @@ void al_free(void *ptr) noexcept _aligned_free(ptr); #else if(ptr != nullptr) - std::free(*(static_cast(ptr) - 1)); + std::free(*(static_cast*>(ptr) - 1)); #endif } diff --git a/common/almalloc.h b/common/almalloc.h index 7ac02bf1..ac4ddfc2 100644 --- a/common/almalloc.h +++ b/common/almalloc.h @@ -13,11 +13,15 @@ #include "pragmadefs.h" -void al_free(void *ptr) noexcept; +namespace gsl { +template using owner = T; +}; + +void al_free(gsl::owner ptr) noexcept; [[gnu::alloc_align(1), gnu::alloc_size(2), gnu::malloc]] -void *al_malloc(size_t alignment, size_t size); +gsl::owner al_malloc(size_t alignment, size_t size); [[gnu::alloc_align(1), gnu::alloc_size(2), gnu::malloc]] -void *al_calloc(size_t alignment, size_t size); +gsl::owner al_calloc(size_t alignment, size_t size); #define DISABLE_ALLOC \ @@ -29,10 +33,10 @@ void *al_calloc(size_t alignment, size_t size); #define DEF_PLACE_NEWDEL \ void *operator new(size_t) = delete; \ void *operator new[](size_t) = delete; \ - void operator delete(void *block, void*) noexcept { al_free(block); } \ - void operator delete(void *block) noexcept { al_free(block); } \ - void operator delete[](void *block, void*) noexcept { al_free(block); } \ - void operator delete[](void *block) noexcept { al_free(block); } + void operator delete(gsl::owner block, void*) noexcept { al_free(block); } \ + void operator delete(gsl::owner block) noexcept { al_free(block); } \ + void operator delete[](gsl::owner block, void*) noexcept { al_free(block); } \ + void operator delete[](gsl::owner block) noexcept { al_free(block); } enum FamCount : size_t { }; @@ -45,15 +49,22 @@ enum FamCount : size_t { }; sizeof(T)); \ } \ \ - void *operator new(size_t /*size*/, FamCount count) \ + gsl::owner operator new(size_t /*size*/, FamCount count) \ { \ - if(void *ret{al_malloc(alignof(T), T::Sizeof(count))}) \ - return ret; \ - throw std::bad_alloc(); \ + const auto align = std::align_val_t(alignof(T)); \ + return ::new(align) std::byte[T::Sizeof(count)]; \ } \ void *operator new[](size_t /*size*/) = delete; \ - void operator delete(void *block, FamCount) noexcept { al_free(block); } \ - void operator delete(void *block) noexcept { al_free(block); } \ + void operator delete(gsl::owner block, FamCount) noexcept \ + { \ + const auto align = std::align_val_t(alignof(T)); \ + ::operator delete[](static_cast>(block), align); \ + } \ + void operator delete(gsl::owner block) noexcept \ + { \ + const auto align = std::align_val_t(alignof(T)); \ + ::operator delete[](static_cast>(block), align); \ + } \ void operator delete[](void* /*block*/) = delete; @@ -61,7 +72,8 @@ namespace al { template struct allocator { - static constexpr std::size_t alignment{std::max(Align, alignof(T))}; + static constexpr auto alignment = std::max(Align, alignof(T)); + static constexpr auto AlignVal = std::align_val_t(alignment); using value_type = T; using reference = T&; @@ -81,13 +93,13 @@ struct allocator { template constexpr explicit allocator(const allocator&) noexcept { } - T *allocate(std::size_t n) + gsl::owner allocate(std::size_t n) { if(n > std::numeric_limits::max()/sizeof(T)) throw std::bad_alloc(); - if(auto p = al_malloc(alignment, n*sizeof(T))) return static_cast(p); - throw std::bad_alloc(); + return reinterpret_cast>(::new(AlignVal) std::byte[n*sizeof(T)]); } - void deallocate(T *p, std::size_t) noexcept { al_free(p); } + void deallocate(gsl::owner p, std::size_t) noexcept + { ::operator delete[](reinterpret_cast>(p), AlignVal); } }; template constexpr bool operator==(const allocator&, const allocator&) noexcept { return true; } @@ -111,8 +123,15 @@ constexpr auto to_address(const T &p) noexcept template constexpr T* construct_at(T *ptr, Args&& ...args) - noexcept(std::is_nothrow_constructible::value) -{ return ::new(static_cast(ptr)) T{std::forward(args)...}; } + noexcept(std::is_nothrow_constructible_v) +{ + /* NOLINTBEGIN(cppcoreguidelines-owning-memory) construct_at doesn't + * necessarily handle the address from an owner, while placement new + * expects to. + */ + return ::new(static_cast(ptr)) T{std::forward(args)...}; + /* NOLINTEND(cppcoreguidelines-owning-memory) */ +} } // namespace al diff --git a/common/flexarray.h b/common/flexarray.h index 9b6fcc63..7975a52a 100644 --- a/common/flexarray.h +++ b/common/flexarray.h @@ -74,7 +74,7 @@ struct FlexArray { { return Storage_t_::Sizeof(count, base); } static std::unique_ptr Create(index_type count) { - if(void *ptr{al_calloc(alignof(FlexArray), Sizeof(count))}) + if(gsl::owner ptr{al_calloc(alignof(FlexArray), Sizeof(count))}) { try { return std::unique_ptr{::new(ptr) FlexArray{count}}; @@ -87,7 +87,7 @@ struct FlexArray { throw std::bad_alloc(); } - FlexArray(index_type size) noexcept(std::is_nothrow_constructible_v) + FlexArray(index_type size) noexcept(std::is_nothrow_constructible_v) : mStore{size} { } ~FlexArray() = default; diff --git a/core/context.cpp b/core/context.cpp index 1415857c..b969583b 100644 --- a/core/context.cpp +++ b/core/context.cpp @@ -29,15 +29,12 @@ ContextBase::~ContextBase() { size_t count{0}; ContextProps *cprops{mParams.ContextUpdate.exchange(nullptr, std::memory_order_relaxed)}; - if(cprops) - { + if(std::unique_ptr old{cprops}) ++count; - delete cprops; - } + cprops = mFreeContextProps.exchange(nullptr, std::memory_order_acquire); - while(cprops) + while(std::unique_ptr old{cprops}) { - std::unique_ptr old{cprops}; cprops = old->next.load(std::memory_order_relaxed); ++count; } @@ -45,21 +42,17 @@ ContextBase::~ContextBase() count = 0; EffectSlotProps *eprops{mFreeEffectslotProps.exchange(nullptr, std::memory_order_acquire)}; - while(eprops) + while(std::unique_ptr old{eprops}) { - std::unique_ptr old{eprops}; eprops = old->next.load(std::memory_order_relaxed); ++count; } TRACE("Freed %zu AuxiliaryEffectSlot property object%s\n", count, (count==1)?"":"s"); - if(EffectSlotArray *curarray{mActiveAuxSlots.exchange(nullptr, std::memory_order_relaxed)}) - { + if(std::unique_ptr curarray{mActiveAuxSlots.exchange(nullptr, std::memory_order_relaxed)}) std::destroy_n(curarray->end(), curarray->size()); - delete curarray; - } - delete mVoices.exchange(nullptr, std::memory_order_relaxed); + std::unique_ptr{mVoices.exchange(nullptr, std::memory_order_relaxed)}; if(mAsyncEvents) { @@ -149,11 +142,8 @@ void ContextBase::allocVoices(size_t addcount) voice_iter = std::transform(cluster->begin(), cluster->end(), voice_iter, [](Voice &voice) noexcept -> Voice* { return &voice; }); - if(auto *oldvoices = mVoices.exchange(newarray.release(), std::memory_order_acq_rel)) - { + if(std::unique_ptr oldvoices{mVoices.exchange(newarray.release(), std::memory_order_acq_rel)}) std::ignore = mDevice->waitForMix(); - delete oldvoices; - } } -- cgit v1.2.3 From 10ecdff7d1dfcc16bd2a090f089781310ea9a93d Mon Sep 17 00:00:00 2001 From: Chris Robinson Date: Fri, 29 Dec 2023 03:39:58 -0800 Subject: Handle pointer ownership a bit better --- al/auxeffectslot.cpp | 30 ++++++++++++------------- al/buffer.cpp | 10 ++++----- al/effect.cpp | 10 ++++----- al/filter.cpp | 10 ++++----- al/source.cpp | 15 +++++++------ alc/alc.cpp | 46 ++++++++++++++++++++++++++------------ alc/backends/wave.cpp | 6 ++--- alc/context.cpp | 4 ++-- alc/context.h | 5 +++-- alc/device.h | 7 +++--- core/mastering.cpp | 13 ++++++----- utils/alsoft-config/mainwindow.cpp | 16 ++++++------- utils/makemhr/makemhr.cpp | 31 +++++++++++++------------ utils/uhjdecoder.cpp | 3 ++- 14 files changed, 115 insertions(+), 91 deletions(-) (limited to 'al/buffer.cpp') diff --git a/al/auxeffectslot.cpp b/al/auxeffectslot.cpp index ea41a842..695c5788 100644 --- a/al/auxeffectslot.cpp +++ b/al/auxeffectslot.cpp @@ -96,7 +96,7 @@ inline ALeffectslot *LookupEffectSlot(ALCcontext *context, ALuint id) noexcept EffectSlotSubList &sublist{context->mEffectSlotList[lidx]}; if(sublist.FreeMask & (1_u64 << slidx)) UNLIKELY return nullptr; - return sublist.EffectSlots + slidx; + return al::to_address(sublist.EffectSlots->begin() + slidx); } inline ALeffect *LookupEffect(ALCdevice *device, ALuint id) noexcept @@ -109,7 +109,7 @@ inline ALeffect *LookupEffect(ALCdevice *device, ALuint id) noexcept EffectSubList &sublist = device->EffectList[lidx]; if(sublist.FreeMask & (1_u64 << slidx)) UNLIKELY return nullptr; - return sublist.Effects + slidx; + return al::to_address(sublist.Effects->begin() + slidx); } inline ALbuffer *LookupBuffer(ALCdevice *device, ALuint id) noexcept @@ -122,7 +122,7 @@ inline ALbuffer *LookupBuffer(ALCdevice *device, ALuint id) noexcept BufferSubList &sublist = device->BufferList[lidx]; if(sublist.FreeMask & (1_u64 << slidx)) UNLIKELY return nullptr; - return sublist.Buffers + slidx; + return al::to_address(sublist.Buffers->begin() + slidx); } @@ -246,8 +246,8 @@ bool EnsureEffectSlots(ALCcontext *context, size_t needed) context->mEffectSlotList.emplace_back(); auto sublist = context->mEffectSlotList.end() - 1; sublist->FreeMask = ~0_u64; - sublist->EffectSlots = static_cast>( - al_calloc(alignof(ALeffectslot), sizeof(ALeffectslot)*64)); + sublist->EffectSlots = static_cast*>>( + al_calloc(alignof(ALeffectslot), sizeof(*sublist->EffectSlots))); if(!sublist->EffectSlots) UNLIKELY { context->mEffectSlotList.pop_back(); @@ -267,7 +267,8 @@ ALeffectslot *AllocEffectSlot(ALCcontext *context) auto slidx = static_cast(al::countr_zero(sublist->FreeMask)); ASSUME(slidx < 64); - ALeffectslot *slot{al::construct_at(sublist->EffectSlots + slidx, context)}; + ALeffectslot *slot{al::construct_at(al::to_address(sublist->EffectSlots->begin() + slidx), + context)}; aluInitEffectPanning(slot->mSlot, context); /* Add 1 to avoid ID 0. */ @@ -875,12 +876,9 @@ ALeffectslot::~ALeffectslot() DecrementRef(Buffer->ref); Buffer = nullptr; - if(EffectSlotProps *props{mSlot->Update.exchange(nullptr)}) - { + if(std::unique_ptr props{mSlot->Update.exchange(nullptr)}) TRACE("Freed unapplied AuxiliaryEffectSlot update %p\n", - decltype(std::declval()){props}); - delete props; - } + decltype(std::declval()){props.get()}); mSlot->mEffectState = nullptr; mSlot->InUse = false; @@ -983,12 +981,12 @@ void UpdateAllEffectSlotProps(ALCcontext *context) uint64_t usemask{~sublist.FreeMask}; while(usemask) { - const int idx{al::countr_zero(usemask)}; + const auto idx = static_cast(al::countr_zero(usemask)); usemask &= ~(1_u64 << idx); - ALeffectslot *slot{sublist.EffectSlots + idx}; + auto &slot = (*sublist.EffectSlots)[idx]; - if(slot->mState != SlotState::Stopped && std::exchange(slot->mPropsDirty, false)) - slot->updateProps(context); + if(slot.mState != SlotState::Stopped && std::exchange(slot.mPropsDirty, false)) + slot.updateProps(context); } } } @@ -1002,7 +1000,7 @@ EffectSlotSubList::~EffectSlotSubList() while(usemask) { const int idx{al::countr_zero(usemask)}; - std::destroy_at(EffectSlots+idx); + std::destroy_at(al::to_address(EffectSlots->begin() + idx)); usemask &= ~(1_u64 << idx); } FreeMask = ~usemask; diff --git a/al/buffer.cpp b/al/buffer.cpp index c0f3f348..46ef8ece 100644 --- a/al/buffer.cpp +++ b/al/buffer.cpp @@ -186,8 +186,8 @@ bool EnsureBuffers(ALCdevice *device, size_t needed) device->BufferList.emplace_back(); auto sublist = device->BufferList.end() - 1; sublist->FreeMask = ~0_u64; - sublist->Buffers = static_cast>(al_calloc(alignof(ALbuffer), - sizeof(ALbuffer)*64)); + sublist->Buffers = static_cast*>>(al_calloc( + alignof(ALbuffer), sizeof(*sublist->Buffers))); if(!sublist->Buffers) UNLIKELY { device->BufferList.pop_back(); @@ -207,7 +207,7 @@ ALbuffer *AllocBuffer(ALCdevice *device) auto slidx = static_cast(al::countr_zero(sublist->FreeMask)); ASSUME(slidx < 64); - ALbuffer *buffer{al::construct_at(sublist->Buffers + slidx)}; + ALbuffer *buffer{al::construct_at(al::to_address(sublist->Buffers->begin() + slidx))}; /* Add 1 to avoid buffer ID 0. */ buffer->id = ((lidx<<6) | slidx) + 1; @@ -244,7 +244,7 @@ inline ALbuffer *LookupBuffer(ALCdevice *device, ALuint id) BufferSubList &sublist = device->BufferList[lidx]; if(sublist.FreeMask & (1_u64 << slidx)) UNLIKELY return nullptr; - return sublist.Buffers + slidx; + return al::to_address(sublist.Buffers->begin() + slidx); } @@ -1486,7 +1486,7 @@ BufferSubList::~BufferSubList() while(usemask) { const int idx{al::countr_zero(usemask)}; - std::destroy_at(Buffers+idx); + std::destroy_at(al::to_address(Buffers->begin() + idx)); usemask &= ~(1_u64 << idx); } FreeMask = ~usemask; diff --git a/al/effect.cpp b/al/effect.cpp index c2a2d1b1..1024de80 100644 --- a/al/effect.cpp +++ b/al/effect.cpp @@ -134,8 +134,8 @@ bool EnsureEffects(ALCdevice *device, size_t needed) device->EffectList.emplace_back(); auto sublist = device->EffectList.end() - 1; sublist->FreeMask = ~0_u64; - sublist->Effects = static_cast>(al_calloc(alignof(ALeffect), - sizeof(ALeffect)*64)); + sublist->Effects = static_cast*>>(al_calloc( + alignof(ALeffect), sizeof(*sublist->Effects))); if(!sublist->Effects) UNLIKELY { device->EffectList.pop_back(); @@ -155,7 +155,7 @@ ALeffect *AllocEffect(ALCdevice *device) auto slidx = static_cast(al::countr_zero(sublist->FreeMask)); ASSUME(slidx < 64); - ALeffect *effect{al::construct_at(sublist->Effects + slidx)}; + ALeffect *effect{al::construct_at(al::to_address(sublist->Effects->begin() + slidx))}; InitEffectParams(effect, AL_EFFECT_NULL); /* Add 1 to avoid effect ID 0. */ @@ -189,7 +189,7 @@ inline ALeffect *LookupEffect(ALCdevice *device, ALuint id) EffectSubList &sublist = device->EffectList[lidx]; if(sublist.FreeMask & (1_u64 << slidx)) UNLIKELY return nullptr; - return sublist.Effects + slidx; + return al::to_address(sublist.Effects->begin() + slidx); } } // namespace @@ -568,7 +568,7 @@ EffectSubList::~EffectSubList() while(usemask) { const int idx{al::countr_zero(usemask)}; - std::destroy_at(Effects+idx); + std::destroy_at(al::to_address(Effects->begin()+idx)); usemask &= ~(1_u64 << idx); } FreeMask = ~usemask; diff --git a/al/filter.cpp b/al/filter.cpp index ce37b0aa..4a24a38f 100644 --- a/al/filter.cpp +++ b/al/filter.cpp @@ -129,8 +129,8 @@ bool EnsureFilters(ALCdevice *device, size_t needed) device->FilterList.emplace_back(); auto sublist = device->FilterList.end() - 1; sublist->FreeMask = ~0_u64; - sublist->Filters = static_cast>(al_calloc(alignof(ALfilter), - sizeof(ALfilter)*64)); + sublist->Filters = static_cast*>>(al_calloc( + alignof(ALfilter), sizeof(*sublist->Filters))); if(!sublist->Filters) UNLIKELY { device->FilterList.pop_back(); @@ -151,7 +151,7 @@ ALfilter *AllocFilter(ALCdevice *device) auto slidx = static_cast(al::countr_zero(sublist->FreeMask)); ASSUME(slidx < 64); - ALfilter *filter{al::construct_at(sublist->Filters + slidx)}; + ALfilter *filter{al::construct_at(al::to_address(sublist->Filters->begin() + slidx))}; InitFilterParams(filter, AL_FILTER_NULL); /* Add 1 to avoid filter ID 0. */ @@ -186,7 +186,7 @@ inline ALfilter *LookupFilter(ALCdevice *device, ALuint id) FilterSubList &sublist = device->FilterList[lidx]; if(sublist.FreeMask & (1_u64 << slidx)) UNLIKELY return nullptr; - return sublist.Filters + slidx; + return al::to_address(sublist.Filters->begin() + slidx); } } // namespace @@ -696,7 +696,7 @@ FilterSubList::~FilterSubList() while(usemask) { const int idx{al::countr_zero(usemask)}; - std::destroy_at(Filters+idx); + std::destroy_at(al::to_address(Filters->begin() + idx)); usemask &= ~(1_u64 << idx); } FreeMask = ~usemask; diff --git a/al/source.cpp b/al/source.cpp index bf96a769..1ac3bf28 100644 --- a/al/source.cpp +++ b/al/source.cpp @@ -729,7 +729,8 @@ bool EnsureSources(ALCcontext *context, size_t needed) context->mSourceList.emplace_back(); auto sublist = context->mSourceList.end() - 1; sublist->FreeMask = ~0_u64; - sublist->Sources = static_cast(al_calloc(alignof(ALsource), sizeof(ALsource)*64)); + sublist->Sources = static_cast*>>(al_calloc( + alignof(ALsource), sizeof(*sublist->Sources))); if(!sublist->Sources) UNLIKELY { context->mSourceList.pop_back(); @@ -749,7 +750,7 @@ ALsource *AllocSource(ALCcontext *context) auto slidx = static_cast(al::countr_zero(sublist->FreeMask)); ASSUME(slidx < 64); - ALsource *source{al::construct_at(sublist->Sources + slidx)}; + ALsource *source{al::construct_at(al::to_address(sublist->Sources->begin() + slidx))}; /* Add 1 to avoid source ID 0. */ source->id = ((lidx<<6) | slidx) + 1; @@ -797,7 +798,7 @@ inline ALsource *LookupSource(ALCcontext *context, ALuint id) noexcept SourceSubList &sublist{context->mSourceList[lidx]}; if(sublist.FreeMask & (1_u64 << slidx)) UNLIKELY return nullptr; - return sublist.Sources + slidx; + return al::to_address(sublist.Sources->begin() + slidx); } auto LookupBuffer = [](ALCdevice *device, auto id) noexcept -> ALbuffer* @@ -810,7 +811,7 @@ auto LookupBuffer = [](ALCdevice *device, auto id) noexcept -> ALbuffer* BufferSubList &sublist = device->BufferList[static_cast(lidx)]; if(sublist.FreeMask & (1_u64 << slidx)) UNLIKELY return nullptr; - return sublist.Buffers + static_cast(slidx); + return al::to_address(sublist.Buffers->begin() + static_cast(slidx)); }; auto LookupFilter = [](ALCdevice *device, auto id) noexcept -> ALfilter* @@ -823,7 +824,7 @@ auto LookupFilter = [](ALCdevice *device, auto id) noexcept -> ALfilter* FilterSubList &sublist = device->FilterList[static_cast(lidx)]; if(sublist.FreeMask & (1_u64 << slidx)) UNLIKELY return nullptr; - return sublist.Filters + static_cast(slidx); + return al::to_address(sublist.Filters->begin() + static_cast(slidx)); }; auto LookupEffectSlot = [](ALCcontext *context, auto id) noexcept -> ALeffectslot* @@ -836,7 +837,7 @@ auto LookupEffectSlot = [](ALCcontext *context, auto id) noexcept -> ALeffectslo EffectSlotSubList &sublist{context->mEffectSlotList[static_cast(lidx)]}; if(sublist.FreeMask & (1_u64 << slidx)) UNLIKELY return nullptr; - return sublist.EffectSlots + static_cast(slidx); + return al::to_address(sublist.EffectSlots->begin() + static_cast(slidx)); }; @@ -3639,7 +3640,7 @@ SourceSubList::~SourceSubList() { const int idx{al::countr_zero(usemask)}; usemask &= ~(1_u64 << idx); - std::destroy_at(Sources+idx); + std::destroy_at(al::to_address(Sources->begin() + idx)); } FreeMask = ~usemask; al_free(Sources); diff --git a/alc/alc.cpp b/alc/alc.cpp index 64b77080..ece65430 100644 --- a/alc/alc.cpp +++ b/alc/alc.cpp @@ -1682,16 +1682,16 @@ ALCenum UpdateDeviceParams(ALCdevice *device, const int *attrList) uint64_t usemask{~sublist.FreeMask}; while(usemask) { - const int idx{al::countr_zero(usemask)}; - ALeffectslot *slot{sublist.EffectSlots + idx}; + const auto idx = static_cast(al::countr_zero(usemask)); + auto &slot = (*sublist.EffectSlots)[idx]; usemask &= ~(1_u64 << idx); - aluInitEffectPanning(slot->mSlot, context); + aluInitEffectPanning(slot.mSlot, context); - EffectState *state{slot->Effect.State.get()}; + EffectState *state{slot.Effect.State.get()}; state->mOutTarget = device->Dry.Buffer; - state->deviceUpdate(device, slot->Buffer); - slot->updateProps(context); + state->deviceUpdate(device, slot.Buffer); + slot.updateProps(context); } } slotlock.unlock(); @@ -1703,8 +1703,8 @@ ALCenum UpdateDeviceParams(ALCdevice *device, const int *attrList) uint64_t usemask{~sublist.FreeMask}; while(usemask) { - const int idx{al::countr_zero(usemask)}; - ALsource *source{sublist.Sources + idx}; + const auto idx = static_cast(al::countr_zero(usemask)); + auto &source = (*sublist.Sources)[idx]; usemask &= ~(1_u64 << idx); auto clear_send = [](ALsource::SendData &send) -> void @@ -1718,10 +1718,10 @@ ALCenum UpdateDeviceParams(ALCdevice *device, const int *attrList) send.GainLF = 1.0f; send.LFReference = HIGHPASSFREQREF; }; - auto send_begin = source->Send.begin() + static_cast(num_sends); - std::for_each(send_begin, source->Send.end(), clear_send); + auto send_begin = source.Send.begin() + static_cast(num_sends); + std::for_each(send_begin, source.Send.end(), clear_send); - source->mPropsDirty = true; + source.mPropsDirty = true; } } @@ -2905,7 +2905,13 @@ ALC_API ALCdevice* ALC_APIENTRY alcOpenDevice(const ALCchar *deviceName) noexcep uint{DefaultSendCount} }; - DeviceRef device{new ALCdevice{DeviceType::Playback}}; + DeviceRef device{new(std::nothrow) ALCdevice{DeviceType::Playback}}; + if(!device) + { + WARN("Failed to create playback device handle\n"); + alcSetError(nullptr, ALC_OUT_OF_MEMORY); + return nullptr; + } /* Set output format */ device->FmtChans = DevFmtChannelsDefault; @@ -3040,7 +3046,13 @@ ALC_API ALCdevice* ALC_APIENTRY alcCaptureOpenDevice(const ALCchar *deviceName, else TRACE("Opening default capture device\n"); - DeviceRef device{new ALCdevice{DeviceType::Capture}}; + DeviceRef device{new(std::nothrow) ALCdevice{DeviceType::Capture}}; + if(!device) + { + WARN("Failed to create capture device handle\n"); + alcSetError(nullptr, ALC_OUT_OF_MEMORY); + return nullptr; + } auto decompfmt = DecomposeDevFormat(format); if(!decompfmt) @@ -3220,7 +3232,13 @@ ALC_API ALCdevice* ALC_APIENTRY alcLoopbackOpenDeviceSOFT(const ALCchar *deviceN uint{DefaultSendCount} }; - DeviceRef device{new ALCdevice{DeviceType::Loopback}}; + DeviceRef device{new(std::nothrow) ALCdevice{DeviceType::Loopback}}; + if(!device) + { + WARN("Failed to create loopback device handle\n"); + alcSetError(nullptr, ALC_OUT_OF_MEMORY); + return nullptr; + } device->SourcesMax = 256; device->AuxiliaryEffectSlotMax = 64; diff --git a/alc/backends/wave.cpp b/alc/backends/wave.cpp index 60cebd7f..985bb539 100644 --- a/alc/backends/wave.cpp +++ b/alc/backends/wave.cpp @@ -56,7 +56,7 @@ using ubyte = unsigned char; using ushort = unsigned short; struct FileDeleter { - void operator()(FILE *f) { fclose(f); } + void operator()(gsl::owner f) { fclose(f); } }; using FilePtr = std::unique_ptr; @@ -211,10 +211,10 @@ void WaveBackend::open(std::string_view name) #ifdef _WIN32 { std::wstring wname{utf8_to_wstr(fname.value())}; - mFile.reset(_wfopen(wname.c_str(), L"wb")); + mFile = FilePtr{_wfopen(wname.c_str(), L"wb")}; } #else - mFile.reset(fopen(fname->c_str(), "wb")); + mFile = FilePtr{fopen(fname->c_str(), "wb")}; #endif if(!mFile) throw al::backend_exception{al::backend_error::DeviceError, "Could not open file '%s': %s", diff --git a/alc/context.cpp b/alc/context.cpp index b5db03f9..0f6dbbae 100644 --- a/alc/context.cpp +++ b/alc/context.cpp @@ -338,10 +338,10 @@ void ForEachSource(ALCcontext *context, F func) uint64_t usemask{~sublist.FreeMask}; while(usemask) { - const int idx{al::countr_zero(usemask)}; + const auto idx = static_cast(al::countr_zero(usemask)); usemask &= ~(1_u64 << idx); - func(sublist.Sources[idx]); + func((*sublist.Sources)[idx]); } } } diff --git a/alc/context.h b/alc/context.h index d5e5e78b..d033c08e 100644 --- a/alc/context.h +++ b/alc/context.h @@ -1,6 +1,7 @@ #ifndef ALC_CONTEXT_H #define ALC_CONTEXT_H +#include #include #include #include @@ -70,7 +71,7 @@ struct DebugLogEntry { struct SourceSubList { uint64_t FreeMask{~0_u64}; - gsl::owner Sources{nullptr}; /* 64 */ + gsl::owner*> Sources{nullptr}; SourceSubList() noexcept = default; SourceSubList(const SourceSubList&) = delete; @@ -85,7 +86,7 @@ struct SourceSubList { struct EffectSlotSubList { uint64_t FreeMask{~0_u64}; - gsl::owner EffectSlots{nullptr}; /* 64 */ + gsl::owner*> EffectSlots{nullptr}; EffectSlotSubList() noexcept = default; EffectSlotSubList(const EffectSlotSubList&) = delete; diff --git a/alc/device.h b/alc/device.h index e5e9b3d2..fe9dff67 100644 --- a/alc/device.h +++ b/alc/device.h @@ -1,6 +1,7 @@ #ifndef ALC_DEVICE_H #define ALC_DEVICE_H +#include #include #include #include @@ -35,7 +36,7 @@ using uint = unsigned int; struct BufferSubList { uint64_t FreeMask{~0_u64}; - gsl::owner Buffers{nullptr}; /* 64 */ + gsl::owner*> Buffers{nullptr}; BufferSubList() noexcept = default; BufferSubList(const BufferSubList&) = delete; @@ -50,7 +51,7 @@ struct BufferSubList { struct EffectSubList { uint64_t FreeMask{~0_u64}; - gsl::owner Effects{nullptr}; /* 64 */ + gsl::owner*> Effects{nullptr}; /* 64 */ EffectSubList() noexcept = default; EffectSubList(const EffectSubList&) = delete; @@ -65,7 +66,7 @@ struct EffectSubList { struct FilterSubList { uint64_t FreeMask{~0_u64}; - gsl::owner Filters{nullptr}; /* 64 */ + gsl::owner*> Filters{nullptr}; FilterSubList() noexcept = default; FilterSubList(const FilterSubList&) = delete; diff --git a/core/mastering.cpp b/core/mastering.cpp index e9b079d6..5fecdb3f 100644 --- a/core/mastering.cpp +++ b/core/mastering.cpp @@ -318,10 +318,10 @@ std::unique_ptr Compressor::Create(const size_t NumChans, const floa const float PostGainDb, const float ThresholdDb, const float Ratio, const float KneeDb, const float AttackTime, const float ReleaseTime) { - const auto lookAhead = static_cast( - clampf(std::round(LookAheadTime*SampleRate), 0.0f, BufferLineSize-1)); - const auto hold = static_cast( - clampf(std::round(HoldTime*SampleRate), 0.0f, BufferLineSize-1)); + const auto lookAhead = static_cast(clampf(std::round(LookAheadTime*SampleRate), 0.0f, + BufferLineSize-1)); + const auto hold = static_cast(clampf(std::round(HoldTime*SampleRate), 0.0f, + BufferLineSize-1)); size_t size{sizeof(Compressor)}; if(lookAhead > 0) @@ -335,7 +335,10 @@ std::unique_ptr Compressor::Create(const size_t NumChans, const floa size += sizeof(*Compressor::mHold); } - auto Comp = CompressorPtr{al::construct_at(static_cast(al_calloc(16, size)))}; + gsl::owner storage{al_calloc(16, size)}; + if(!storage) throw std::bad_alloc{}; + + auto Comp = CompressorPtr{::new(storage) Compressor{}}; Comp->mNumChans = NumChans; Comp->mAuto.Knee = AutoKnee; Comp->mAuto.Attack = AutoAttack; diff --git a/utils/alsoft-config/mainwindow.cpp b/utils/alsoft-config/mainwindow.cpp index b2412c73..9a65f79c 100644 --- a/utils/alsoft-config/mainwindow.cpp +++ b/utils/alsoft-config/mainwindow.cpp @@ -20,6 +20,7 @@ #include #endif +#include "almalloc.h" #include "alspan.h" namespace { @@ -1283,11 +1284,10 @@ void MainWindow::addHrtfFile() void MainWindow::removeHrtfFile() { - QList selected{ui->hrtfFileList->selectedItems()}; + QList> selected{ui->hrtfFileList->selectedItems()}; if(!selected.isEmpty()) { - foreach(QListWidgetItem *item, selected) - delete item; + std::for_each(selected.begin(), selected.end(), std::default_delete{}); enableApplyButton(); } } @@ -1321,9 +1321,8 @@ void MainWindow::showEnabledBackendMenu(QPoint pt) QAction *gotAction{ctxmenu.exec(pt)}; if(gotAction == removeAction) { - QList selected{ui->enabledBackendList->selectedItems()}; - foreach(QListWidgetItem *item, selected) - delete item; + QList> selected{ui->enabledBackendList->selectedItems()}; + std::for_each(selected.begin(), selected.end(), std::default_delete{}); enableApplyButton(); } else if(gotAction != nullptr) @@ -1359,9 +1358,8 @@ void MainWindow::showDisabledBackendMenu(QPoint pt) QAction *gotAction{ctxmenu.exec(pt)}; if(gotAction == removeAction) { - QList selected{ui->disabledBackendList->selectedItems()}; - foreach(QListWidgetItem *item, selected) - delete item; + QList> selected{ui->disabledBackendList->selectedItems()}; + std::for_each(selected.begin(), selected.end(), std::default_delete{}); enableApplyButton(); } else if(gotAction != nullptr) diff --git a/utils/makemhr/makemhr.cpp b/utils/makemhr/makemhr.cpp index f14110c0..0a9b71e7 100644 --- a/utils/makemhr/makemhr.cpp +++ b/utils/makemhr/makemhr.cpp @@ -104,6 +104,11 @@ HrirDataT::~HrirDataT() = default; namespace { +struct FileDeleter { + void operator()(gsl::owner f) { fclose(f); } +}; +using FilePtr = std::unique_ptr; + using namespace std::placeholders; // The epsilon used to maintain signal stability. @@ -312,7 +317,6 @@ static int WriteAscii(const char *out, FILE *fp, const char *filename) len = strlen(out); if(fwrite(out, 1, len, fp) != len) { - fclose(fp); fprintf(stderr, "\nError: Bad write to file '%s'.\n", filename); return 0; } @@ -343,33 +347,33 @@ static int StoreMhr(const HrirDataT *hData, const char *filename) uint dither_seed{22222}; uint fi, ei, ai, i; - FILE *fp{fopen(filename, "wb")}; + FilePtr fp{fopen(filename, "wb")}; if(!fp) { fprintf(stderr, "\nError: Could not open MHR file '%s'.\n", filename); return 0; } - if(!WriteAscii(MHRFormat, fp, filename)) + if(!WriteAscii(MHRFormat, fp.get(), filename)) return 0; - if(!WriteBin4(4, hData->mIrRate, fp, filename)) + if(!WriteBin4(4, hData->mIrRate, fp.get(), filename)) return 0; - if(!WriteBin4(1, static_cast(hData->mChannelType), fp, filename)) + if(!WriteBin4(1, static_cast(hData->mChannelType), fp.get(), filename)) return 0; - if(!WriteBin4(1, hData->mIrPoints, fp, filename)) + if(!WriteBin4(1, hData->mIrPoints, fp.get(), filename)) return 0; - if(!WriteBin4(1, static_cast(hData->mFds.size()), fp, filename)) + if(!WriteBin4(1, static_cast(hData->mFds.size()), fp.get(), filename)) return 0; for(fi = static_cast(hData->mFds.size()-1);fi < hData->mFds.size();fi--) { auto fdist = static_cast(std::round(1000.0 * hData->mFds[fi].mDistance)); - if(!WriteBin4(2, fdist, fp, filename)) + if(!WriteBin4(2, fdist, fp.get(), filename)) return 0; - if(!WriteBin4(1, static_cast(hData->mFds[fi].mEvs.size()), fp, filename)) + if(!WriteBin4(1, static_cast(hData->mFds[fi].mEvs.size()), fp.get(), filename)) return 0; for(ei = 0;ei < hData->mFds[fi].mEvs.size();ei++) { const auto &elev = hData->mFds[fi].mEvs[ei]; - if(!WriteBin4(1, static_cast(elev.mAzs.size()), fp, filename)) + if(!WriteBin4(1, static_cast(elev.mAzs.size()), fp.get(), filename)) return 0; } } @@ -392,7 +396,7 @@ static int StoreMhr(const HrirDataT *hData, const char *filename) for(i = 0;i < (channels * n);i++) { const auto v = static_cast(Clamp(out[i], -scale-1.0, scale)); - if(!WriteBin4(bps, static_cast(v), fp, filename)) + if(!WriteBin4(bps, static_cast(v), fp.get(), filename)) return 0; } } @@ -407,16 +411,15 @@ static int StoreMhr(const HrirDataT *hData, const char *filename) for(const auto &azd : hData->mFds[fi].mEvs[ei].mAzs) { auto v = static_cast(std::round(azd.mDelays[0]*DelayPrecScale)); - if(!WriteBin4(1, v, fp, filename)) return 0; + if(!WriteBin4(1, v, fp.get(), filename)) return 0; if(hData->mChannelType == CT_STEREO) { v = static_cast(std::round(azd.mDelays[1]*DelayPrecScale)); - if(!WriteBin4(1, v, fp, filename)) return 0; + if(!WriteBin4(1, v, fp.get(), filename)) return 0; } } } } - fclose(fp); return 1; } diff --git a/utils/uhjdecoder.cpp b/utils/uhjdecoder.cpp index 8f0d2006..970d9f82 100644 --- a/utils/uhjdecoder.cpp +++ b/utils/uhjdecoder.cpp @@ -35,6 +35,7 @@ #include "albit.h" #include "alcomplex.h" +#include "almalloc.h" #include "alnumbers.h" #include "alspan.h" #include "vector.h" @@ -47,7 +48,7 @@ struct FileDeleter { - void operator()(FILE *file) { fclose(file); } + void operator()(gsl::owner file) { fclose(file); } }; using FilePtr = std::unique_ptr; -- cgit v1.2.3 From 2f2edb326128c56e269a171961d991d8d7936e4f Mon Sep 17 00:00:00 2001 From: Chris Robinson Date: Mon, 1 Jan 2024 00:39:33 -0800 Subject: Use standard operator new[] and delete[] for aligned allocations --- CMakeLists.txt | 2 -- al/auxeffectslot.cpp | 2 +- al/buffer.cpp | 2 +- al/effect.cpp | 2 +- al/filter.cpp | 2 +- al/source.cpp | 2 +- common/almalloc.cpp | 50 ++------------------------------------------------ common/almalloc.h | 5 ++--- config.h.in | 6 ------ 9 files changed, 9 insertions(+), 64 deletions(-) (limited to 'al/buffer.cpp') diff --git a/CMakeLists.txt b/CMakeLists.txt index f53236fb..14ee18f5 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -565,8 +565,6 @@ if(HAVE_INTRIN_H) }" HAVE_CPUID_INTRINSIC) endif() -check_symbol_exists(posix_memalign stdlib.h HAVE_POSIX_MEMALIGN) -check_symbol_exists(_aligned_malloc malloc.h HAVE__ALIGNED_MALLOC) check_symbol_exists(proc_pidpath libproc.h HAVE_PROC_PIDPATH) if(NOT WIN32) diff --git a/al/auxeffectslot.cpp b/al/auxeffectslot.cpp index 695c5788..d95748d7 100644 --- a/al/auxeffectslot.cpp +++ b/al/auxeffectslot.cpp @@ -1004,7 +1004,7 @@ EffectSlotSubList::~EffectSlotSubList() usemask &= ~(1_u64 << idx); } FreeMask = ~usemask; - al_free(EffectSlots); + al_free(alignof(ALeffectslot), EffectSlots); EffectSlots = nullptr; } diff --git a/al/buffer.cpp b/al/buffer.cpp index 46ef8ece..646ec8ea 100644 --- a/al/buffer.cpp +++ b/al/buffer.cpp @@ -1490,7 +1490,7 @@ BufferSubList::~BufferSubList() usemask &= ~(1_u64 << idx); } FreeMask = ~usemask; - al_free(Buffers); + al_free(alignof(ALbuffer), Buffers); Buffers = nullptr; } diff --git a/al/effect.cpp b/al/effect.cpp index 1024de80..2f5422fd 100644 --- a/al/effect.cpp +++ b/al/effect.cpp @@ -572,7 +572,7 @@ EffectSubList::~EffectSubList() usemask &= ~(1_u64 << idx); } FreeMask = ~usemask; - al_free(Effects); + al_free(alignof(ALeffect), Effects); Effects = nullptr; } diff --git a/al/filter.cpp b/al/filter.cpp index 0a999169..b67a65f7 100644 --- a/al/filter.cpp +++ b/al/filter.cpp @@ -700,6 +700,6 @@ FilterSubList::~FilterSubList() usemask &= ~(1_u64 << idx); } FreeMask = ~usemask; - al_free(Filters); + al_free(alignof(ALfilter), Filters); Filters = nullptr; } diff --git a/al/source.cpp b/al/source.cpp index af58379b..425acbfa 100644 --- a/al/source.cpp +++ b/al/source.cpp @@ -3643,7 +3643,7 @@ SourceSubList::~SourceSubList() std::destroy_at(al::to_address(Sources->begin() + idx)); } FreeMask = ~usemask; - al_free(Sources); + al_free(alignof(ALsource), Sources); Sources = nullptr; } diff --git a/common/almalloc.cpp b/common/almalloc.cpp index 71953d8b..2249b988 100644 --- a/common/almalloc.cpp +++ b/common/almalloc.cpp @@ -3,59 +3,13 @@ #include "almalloc.h" -#include -#include -#include +#include #include -#include -#ifdef HAVE_MALLOC_H -#include -#endif -gsl::owner al_malloc(size_t alignment, size_t size) -{ - assert((alignment & (alignment-1)) == 0); - alignment = std::max(alignment, alignof(std::max_align_t)); - -#if defined(HAVE_POSIX_MEMALIGN) - gsl::owner ret{}; - if(posix_memalign(&ret, alignment, size) == 0) - return ret; - return nullptr; -#elif defined(HAVE__ALIGNED_MALLOC) - return _aligned_malloc(size, alignment); -#else - size_t total_size{size + alignment-1 + sizeof(void*)}; - void *base{std::malloc(total_size)}; - if(base != nullptr) - { - void *aligned_ptr{static_cast(base) + sizeof(void*)}; - total_size -= sizeof(void*); - - std::align(alignment, size, aligned_ptr, total_size); - *(static_cast(aligned_ptr)-1) = base; - base = aligned_ptr; - } - return base; -#endif -} - gsl::owner al_calloc(size_t alignment, size_t size) { - gsl::owner ret{al_malloc(alignment, size)}; + gsl::owner ret{::operator new[](size, std::align_val_t{alignment}, std::nothrow)}; if(ret) std::memset(ret, 0, size); return ret; } - -void al_free(gsl::owner ptr) noexcept -{ -#if defined(HAVE_POSIX_MEMALIGN) - std::free(ptr); -#elif defined(HAVE__ALIGNED_MALLOC) - _aligned_free(ptr); -#else - if(ptr != nullptr) - std::free(*(static_cast*>(ptr) - 1)); -#endif -} diff --git a/common/almalloc.h b/common/almalloc.h index 82f050e4..a03a0f43 100644 --- a/common/almalloc.h +++ b/common/almalloc.h @@ -17,9 +17,8 @@ namespace gsl { template using owner = T; }; -void al_free(gsl::owner ptr) noexcept; -[[gnu::alloc_align(1), gnu::alloc_size(2), gnu::malloc]] -gsl::owner al_malloc(size_t alignment, size_t size); +inline void al_free(size_t alignment, gsl::owner ptr) noexcept +{ ::operator delete[](ptr, std::align_val_t{alignment}); } [[gnu::alloc_align(1), gnu::alloc_size(2), gnu::malloc]] gsl::owner al_calloc(size_t alignment, size_t size); diff --git a/config.h.in b/config.h.in index 20df5b46..19cb79e0 100644 --- a/config.h.in +++ b/config.h.in @@ -7,12 +7,6 @@ /* Define if HRTF data is embedded in the library */ #cmakedefine ALSOFT_EMBED_HRTF_DATA -/* Define if we have the posix_memalign function */ -#cmakedefine HAVE_POSIX_MEMALIGN - -/* Define if we have the _aligned_malloc function */ -#cmakedefine HAVE__ALIGNED_MALLOC - /* Define if we have the proc_pidpath function */ #cmakedefine HAVE_PROC_PIDPATH -- cgit v1.2.3 From 45fca6a301a6d424bd0d856d0ae29ce56c5dc9ac Mon Sep 17 00:00:00 2001 From: Chris Robinson Date: Mon, 1 Jan 2024 05:11:53 -0800 Subject: Use an allocator to allocate uninitilized sublists --- CMakeLists.txt | 1 - al/auxeffectslot.cpp | 31 ++++++++++++++++--------------- al/buffer.cpp | 31 ++++++++++++++++--------------- al/effect.cpp | 31 ++++++++++++++++--------------- al/filter.cpp | 31 ++++++++++++++++--------------- al/source.cpp | 31 +++++++++++++++---------------- common/almalloc.cpp | 15 --------------- common/almalloc.h | 19 +++++++------------ 8 files changed, 86 insertions(+), 104 deletions(-) delete mode 100644 common/almalloc.cpp (limited to 'al/buffer.cpp') diff --git a/CMakeLists.txt b/CMakeLists.txt index 14ee18f5..b4ced226 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -610,7 +610,6 @@ set(COMMON_OBJS common/alcomplex.h common/alfstream.cpp common/alfstream.h - common/almalloc.cpp common/almalloc.h common/alnumbers.h common/alnumeric.h diff --git a/al/auxeffectslot.cpp b/al/auxeffectslot.cpp index d95748d7..5eea3425 100644 --- a/al/auxeffectslot.cpp +++ b/al/auxeffectslot.cpp @@ -53,6 +53,8 @@ namespace { +using SubListAllocator = typename al::allocator>; + struct FactoryItem { EffectSlotType Type; EffectStateFactory* (&GetFactory)(); @@ -238,22 +240,21 @@ bool EnsureEffectSlots(ALCcontext *context, size_t needed) [](size_t cur, const EffectSlotSubList &sublist) noexcept -> size_t { return cur + static_cast(al::popcount(sublist.FreeMask)); })}; - while(needed > count) - { - if(context->mEffectSlotList.size() >= 1<<25) UNLIKELY - return false; - - context->mEffectSlotList.emplace_back(); - auto sublist = context->mEffectSlotList.end() - 1; - sublist->FreeMask = ~0_u64; - sublist->EffectSlots = static_cast*>>( - al_calloc(alignof(ALeffectslot), sizeof(*sublist->EffectSlots))); - if(!sublist->EffectSlots) UNLIKELY + try { + while(needed > count) { - context->mEffectSlotList.pop_back(); - return false; + if(context->mEffectSlotList.size() >= 1<<25) UNLIKELY + return false; + + EffectSlotSubList sublist{}; + sublist.FreeMask = ~0_u64; + sublist.EffectSlots = SubListAllocator{}.allocate(1); + context->mEffectSlotList.emplace_back(std::move(sublist)); + count += 64; } - count += 64; + } + catch(...) { + return false; } return true; } @@ -1004,7 +1005,7 @@ EffectSlotSubList::~EffectSlotSubList() usemask &= ~(1_u64 << idx); } FreeMask = ~usemask; - al_free(alignof(ALeffectslot), EffectSlots); + SubListAllocator{}.deallocate(EffectSlots, 1); EffectSlots = nullptr; } diff --git a/al/buffer.cpp b/al/buffer.cpp index 646ec8ea..2aafbf2d 100644 --- a/al/buffer.cpp +++ b/al/buffer.cpp @@ -68,6 +68,8 @@ namespace { +using SubListAllocator = typename al::allocator>; + std::optional AmbiLayoutFromEnum(ALenum layout) { switch(layout) @@ -178,22 +180,21 @@ bool EnsureBuffers(ALCdevice *device, size_t needed) [](size_t cur, const BufferSubList &sublist) noexcept -> size_t { return cur + static_cast(al::popcount(sublist.FreeMask)); })}; - while(needed > count) - { - if(device->BufferList.size() >= 1<<25) UNLIKELY - return false; - - device->BufferList.emplace_back(); - auto sublist = device->BufferList.end() - 1; - sublist->FreeMask = ~0_u64; - sublist->Buffers = static_cast*>>(al_calloc( - alignof(ALbuffer), sizeof(*sublist->Buffers))); - if(!sublist->Buffers) UNLIKELY + try { + while(needed > count) { - device->BufferList.pop_back(); - return false; + if(device->BufferList.size() >= 1<<25) UNLIKELY + return false; + + BufferSubList sublist{}; + sublist.FreeMask = ~0_u64; + sublist.Buffers = SubListAllocator{}.allocate(1); + device->BufferList.emplace_back(std::move(sublist)); + count += 64; } - count += 64; + } + catch(...) { + return false; } return true; } @@ -1490,7 +1491,7 @@ BufferSubList::~BufferSubList() usemask &= ~(1_u64 << idx); } FreeMask = ~usemask; - al_free(alignof(ALbuffer), Buffers); + SubListAllocator{}.deallocate(Buffers, 1); Buffers = nullptr; } diff --git a/al/effect.cpp b/al/effect.cpp index 2f5422fd..071b32c6 100644 --- a/al/effect.cpp +++ b/al/effect.cpp @@ -89,6 +89,8 @@ effect_exception::~effect_exception() = default; namespace { +using SubListAllocator = typename al::allocator>; + auto GetDefaultProps(ALenum type) -> const EffectProps& { switch(type) @@ -126,22 +128,21 @@ bool EnsureEffects(ALCdevice *device, size_t needed) [](size_t cur, const EffectSubList &sublist) noexcept -> size_t { return cur + static_cast(al::popcount(sublist.FreeMask)); })}; - while(needed > count) - { - if(device->EffectList.size() >= 1<<25) UNLIKELY - return false; - - device->EffectList.emplace_back(); - auto sublist = device->EffectList.end() - 1; - sublist->FreeMask = ~0_u64; - sublist->Effects = static_cast*>>(al_calloc( - alignof(ALeffect), sizeof(*sublist->Effects))); - if(!sublist->Effects) UNLIKELY + try { + while(needed > count) { - device->EffectList.pop_back(); - return false; + if(device->EffectList.size() >= 1<<25) UNLIKELY + return false; + + EffectSubList sublist{}; + sublist.FreeMask = ~0_u64; + sublist.Effects = SubListAllocator{}.allocate(1); + device->EffectList.emplace_back(std::move(sublist)); + count += 64; } - count += 64; + } + catch(...) { + return false; } return true; } @@ -572,7 +573,7 @@ EffectSubList::~EffectSubList() usemask &= ~(1_u64 << idx); } FreeMask = ~usemask; - al_free(alignof(ALeffect), Effects); + SubListAllocator{}.deallocate(Effects, 1); Effects = nullptr; } diff --git a/al/filter.cpp b/al/filter.cpp index b67a65f7..528d6059 100644 --- a/al/filter.cpp +++ b/al/filter.cpp @@ -49,6 +49,8 @@ namespace { +using SubListAllocator = typename al::allocator>; + class filter_exception final : public al::base_exception { ALenum mErrorCode; @@ -121,22 +123,21 @@ bool EnsureFilters(ALCdevice *device, size_t needed) [](size_t cur, const FilterSubList &sublist) noexcept -> size_t { return cur + static_cast(al::popcount(sublist.FreeMask)); })}; - while(needed > count) - { - if(device->FilterList.size() >= 1<<25) UNLIKELY - return false; - - device->FilterList.emplace_back(); - auto sublist = device->FilterList.end() - 1; - sublist->FreeMask = ~0_u64; - sublist->Filters = static_cast*>>(al_calloc( - alignof(ALfilter), sizeof(*sublist->Filters))); - if(!sublist->Filters) UNLIKELY + try { + while(needed > count) { - device->FilterList.pop_back(); - return false; + if(device->FilterList.size() >= 1<<25) UNLIKELY + return false; + + FilterSubList sublist{}; + sublist.FreeMask = ~0_u64; + sublist.Filters = SubListAllocator{}.allocate(1); + device->FilterList.emplace_back(std::move(sublist)); + count += 64; } - count += 64; + } + catch(...) { + return false; } return true; } @@ -700,6 +701,6 @@ FilterSubList::~FilterSubList() usemask &= ~(1_u64 << idx); } FreeMask = ~usemask; - al_free(alignof(ALfilter), Filters); + SubListAllocator{}.deallocate(Filters, 1); Filters = nullptr; } diff --git a/al/source.cpp b/al/source.cpp index 425acbfa..27144389 100644 --- a/al/source.cpp +++ b/al/source.cpp @@ -80,7 +80,7 @@ namespace { -using namespace std::placeholders; +using SubListAllocator = typename al::allocator>; using std::chrono::nanoseconds; using seconds_d = std::chrono::duration; @@ -721,22 +721,21 @@ bool EnsureSources(ALCcontext *context, size_t needed) [](size_t cur, const SourceSubList &sublist) noexcept -> size_t { return cur + static_cast(al::popcount(sublist.FreeMask)); })}; - while(needed > count) - { - if(context->mSourceList.size() >= 1<<25) UNLIKELY - return false; - - context->mSourceList.emplace_back(); - auto sublist = context->mSourceList.end() - 1; - sublist->FreeMask = ~0_u64; - sublist->Sources = static_cast*>>(al_calloc( - alignof(ALsource), sizeof(*sublist->Sources))); - if(!sublist->Sources) UNLIKELY + try { + while(needed > count) { - context->mSourceList.pop_back(); - return false; + if(context->mSourceList.size() >= 1<<25) UNLIKELY + return false; + + SourceSubList sublist{}; + sublist.FreeMask = ~0_u64; + sublist.Sources = SubListAllocator{}.allocate(1); + context->mSourceList.emplace_back(std::move(sublist)); + count += 64; } - count += 64; + } + catch(...) { + return false; } return true; } @@ -3643,7 +3642,7 @@ SourceSubList::~SourceSubList() std::destroy_at(al::to_address(Sources->begin() + idx)); } FreeMask = ~usemask; - al_free(alignof(ALsource), Sources); + SubListAllocator{}.deallocate(Sources, 1); Sources = nullptr; } diff --git a/common/almalloc.cpp b/common/almalloc.cpp deleted file mode 100644 index 2249b988..00000000 --- a/common/almalloc.cpp +++ /dev/null @@ -1,15 +0,0 @@ - -#include "config.h" - -#include "almalloc.h" - -#include -#include - - -gsl::owner al_calloc(size_t alignment, size_t size) -{ - gsl::owner ret{::operator new[](size, std::align_val_t{alignment}, std::nothrow)}; - if(ret) std::memset(ret, 0, size); - return ret; -} diff --git a/common/almalloc.h b/common/almalloc.h index a03a0f43..3b9965e6 100644 --- a/common/almalloc.h +++ b/common/almalloc.h @@ -17,11 +17,6 @@ namespace gsl { template using owner = T; }; -inline void al_free(size_t alignment, gsl::owner ptr) noexcept -{ ::operator delete[](ptr, std::align_val_t{alignment}); } -[[gnu::alloc_align(1), gnu::alloc_size(2), gnu::malloc]] -gsl::owner al_calloc(size_t alignment, size_t size); - #define DISABLE_ALLOC \ void *operator new(size_t) = delete; \ @@ -61,18 +56,18 @@ struct allocator { static constexpr auto Alignment = std::max(AlignV, alignof(T)); static constexpr auto AlignVal = std::align_val_t{Alignment}; - using value_type = T; - using reference = T&; - using const_reference = const T&; - using pointer = T*; - using const_pointer = const T*; + using value_type = std::remove_cv_t>; + using reference = value_type&; + using const_reference = const value_type&; + using pointer = value_type*; + using const_pointer = const value_type*; using size_type = std::size_t; using difference_type = std::ptrdiff_t; using is_always_equal = std::true_type; - template + template = true> struct rebind { - using other = std::enable_if_t>; + using other = allocator; }; constexpr explicit allocator() noexcept = default; -- cgit v1.2.3 From 18349e1da2c79d0f41c8c4f12ccd065f91618a6f Mon Sep 17 00:00:00 2001 From: Chris Robinson Date: Wed, 3 Jan 2024 14:58:07 -0800 Subject: Avoid using bit_cast for pointer types --- al/buffer.cpp | 2 +- al/state.cpp | 4 ++-- alc/alc.cpp | 2 +- alc/backends/alsa.cpp | 18 ++++++------------ alc/backends/dsound.cpp | 2 +- alc/backends/jack.cpp | 20 +++++++------------- alc/backends/pipewire.cpp | 8 ++++---- alc/backends/portaudio.cpp | 9 ++++----- alc/backends/pulseaudio.cpp | 13 ++++--------- common/dynload.cpp | 7 +++---- core/dbus_wrap.cpp | 18 +++++++++--------- 11 files changed, 42 insertions(+), 61 deletions(-) (limited to 'al/buffer.cpp') diff --git a/al/buffer.cpp b/al/buffer.cpp index 2aafbf2d..b7ed5b32 100644 --- a/al/buffer.cpp +++ b/al/buffer.cpp @@ -1373,7 +1373,7 @@ FORCE_ALIGN void AL_APIENTRY alGetBufferPtrDirectSOFT(ALCcontext *context, ALuin else switch(param) { case AL_BUFFER_CALLBACK_FUNCTION_SOFT: - *value = al::bit_cast(albuf->mCallback); + *value = reinterpret_cast(albuf->mCallback); break; case AL_BUFFER_CALLBACK_USER_PARAM_SOFT: *value = albuf->mUserData; diff --git a/al/state.cpp b/al/state.cpp index c1e3d593..3ef87fbb 100644 --- a/al/state.cpp +++ b/al/state.cpp @@ -463,7 +463,7 @@ FORCE_ALIGN void AL_APIENTRY alGetPointervDirectSOFT(ALCcontext *context, ALenum switch(pname) { case AL_EVENT_CALLBACK_FUNCTION_SOFT: - *values = al::bit_cast(context->mEventCb); + *values = reinterpret_cast(context->mEventCb); break; case AL_EVENT_CALLBACK_USER_PARAM_SOFT: @@ -471,7 +471,7 @@ FORCE_ALIGN void AL_APIENTRY alGetPointervDirectSOFT(ALCcontext *context, ALenum break; case AL_DEBUG_CALLBACK_FUNCTION_EXT: - *values = al::bit_cast(context->mDebugCb); + *values = reinterpret_cast(context->mDebugCb); break; case AL_DEBUG_CALLBACK_USER_PARAM_EXT: diff --git a/alc/alc.cpp b/alc/alc.cpp index e5f2c545..b63317e7 100644 --- a/alc/alc.cpp +++ b/alc/alc.cpp @@ -174,7 +174,7 @@ BOOL APIENTRY DllMain(HINSTANCE module, DWORD reason, LPVOID /*reserved*/) case DLL_PROCESS_ATTACH: /* Pin the DLL so we won't get unloaded until the process terminates */ GetModuleHandleExW(GET_MODULE_HANDLE_EX_FLAG_PIN | GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS, - al::bit_cast(module), &module); + reinterpret_cast(module), &module); break; } return TRUE; diff --git a/alc/backends/alsa.cpp b/alc/backends/alsa.cpp index d7dd16c0..4bda5b02 100644 --- a/alc/backends/alsa.cpp +++ b/alc/backends/alsa.cpp @@ -1196,13 +1196,9 @@ ClockLatency AlsaCapture::getClockLatency() bool AlsaBackendFactory::init() { - bool error{false}; - #ifdef HAVE_DYNLOAD if(!alsa_handle) { - std::string missing_funcs; - alsa_handle = LoadLib("libasound.so.2"); if(!alsa_handle) { @@ -1210,27 +1206,25 @@ bool AlsaBackendFactory::init() return false; } - error = false; + std::string missing_funcs; #define LOAD_FUNC(f) do { \ - p##f = al::bit_cast(GetSymbol(alsa_handle, #f)); \ - if(p##f == nullptr) { \ - error = true; \ - missing_funcs += "\n" #f; \ - } \ + p##f = reinterpret_cast(GetSymbol(alsa_handle, #f)); \ + if(p##f == nullptr) missing_funcs += "\n" #f; \ } while(0) ALSA_FUNCS(LOAD_FUNC); #undef LOAD_FUNC - if(error) + if(!missing_funcs.empty()) { WARN("Missing expected functions:%s\n", missing_funcs.c_str()); CloseLib(alsa_handle); alsa_handle = nullptr; + return false; } } #endif - return !error; + return true; } bool AlsaBackendFactory::querySupport(BackendType type) diff --git a/alc/backends/dsound.cpp b/alc/backends/dsound.cpp index 59a59a9f..e51c7ab5 100644 --- a/alc/backends/dsound.cpp +++ b/alc/backends/dsound.cpp @@ -774,7 +774,7 @@ bool DSoundBackendFactory::init() } #define LOAD_FUNC(f) do { \ - p##f = al::bit_cast(GetSymbol(ds_handle, #f)); \ + p##f = reinterpret_cast(GetSymbol(ds_handle, #f)); \ if(!p##f) \ { \ CloseLib(ds_handle); \ diff --git a/alc/backends/jack.cpp b/alc/backends/jack.cpp index 922873b8..1e5c17ad 100644 --- a/alc/backends/jack.cpp +++ b/alc/backends/jack.cpp @@ -109,13 +109,9 @@ jack_options_t ClientOptions = JackNullOption; bool jack_load() { - bool error{false}; - #ifdef HAVE_DYNLOAD if(!jack_handle) { - std::string missing_funcs; - #ifdef _WIN32 #define JACKLIB "libjack.dll" #else @@ -128,31 +124,29 @@ bool jack_load() return false; } - error = false; + std::string missing_funcs; #define LOAD_FUNC(f) do { \ - p##f = al::bit_cast(GetSymbol(jack_handle, #f)); \ - if(p##f == nullptr) { \ - error = true; \ - missing_funcs += "\n" #f; \ - } \ + p##f = reinterpret_cast(GetSymbol(jack_handle, #f)); \ + if(p##f == nullptr) missing_funcs += "\n" #f; \ } while(0) JACK_FUNCS(LOAD_FUNC); #undef LOAD_FUNC /* Optional symbols. These don't exist in all versions of JACK. */ -#define LOAD_SYM(f) p##f = al::bit_cast(GetSymbol(jack_handle, #f)) +#define LOAD_SYM(f) p##f = reinterpret_cast(GetSymbol(jack_handle, #f)) LOAD_SYM(jack_error_callback); #undef LOAD_SYM - if(error) + if(!missing_funcs.empty()) { WARN("Missing expected functions:%s\n", missing_funcs.c_str()); CloseLib(jack_handle); jack_handle = nullptr; + return false; } } #endif - return !error; + return true; } diff --git a/alc/backends/pipewire.cpp b/alc/backends/pipewire.cpp index c31f943e..55bcf6f4 100644 --- a/alc/backends/pipewire.cpp +++ b/alc/backends/pipewire.cpp @@ -256,7 +256,7 @@ bool pwire_load() } #define LOAD_FUNC(f) do { \ - p##f = al::bit_cast(GetSymbol(pwire_handle, #f)); \ + p##f = reinterpret_cast(GetSymbol(pwire_handle, #f)); \ if(p##f == nullptr) missing_funcs += "\n" #f; \ } while(0); PWIRE_FUNCS(LOAD_FUNC) @@ -374,11 +374,11 @@ To as(From) noexcept = delete; * - pw_metadata */ template<> -pw_proxy* as(pw_registry *reg) noexcept { return al::bit_cast(reg); } +pw_proxy* as(pw_registry *reg) noexcept { return reinterpret_cast(reg); } template<> -pw_proxy* as(pw_node *node) noexcept { return al::bit_cast(node); } +pw_proxy* as(pw_node *node) noexcept { return reinterpret_cast(node); } template<> -pw_proxy* as(pw_metadata *mdata) noexcept { return al::bit_cast(mdata); } +pw_proxy* as(pw_metadata *mdata) noexcept { return reinterpret_cast(mdata); } struct PwContextDeleter { diff --git a/alc/backends/portaudio.cpp b/alc/backends/portaudio.cpp index a8bd00fd..7c61e134 100644 --- a/alc/backends/portaudio.cpp +++ b/alc/backends/portaudio.cpp @@ -349,8 +349,6 @@ void PortCapture::captureSamples(std::byte *buffer, uint samples) bool PortBackendFactory::init() { - PaError err; - #ifdef HAVE_DYNLOAD if(!pa_handle) { @@ -369,7 +367,7 @@ bool PortBackendFactory::init() return false; #define LOAD_FUNC(f) do { \ - p##f = al::bit_cast(GetSymbol(pa_handle, #f)); \ + p##f = reinterpret_cast(GetSymbol(pa_handle, #f)); \ if(p##f == nullptr) \ { \ CloseLib(pa_handle); \ @@ -389,7 +387,7 @@ bool PortBackendFactory::init() LOAD_FUNC(Pa_GetStreamInfo); #undef LOAD_FUNC - err = Pa_Initialize(); + const PaError err{Pa_Initialize()}; if(err != paNoError) { ERR("Pa_Initialize() returned an error: %s\n", Pa_GetErrorText(err)); @@ -399,7 +397,8 @@ bool PortBackendFactory::init() } } #else - if((err=Pa_Initialize()) != paNoError) + const PaError err{Pa_Initialize()}; + if(err != paNoError) { ERR("Pa_Initialize() returned an error: %s\n", Pa_GetErrorText(err)); return false; diff --git a/alc/backends/pulseaudio.cpp b/alc/backends/pulseaudio.cpp index 77d45466..e976fc27 100644 --- a/alc/backends/pulseaudio.cpp +++ b/alc/backends/pulseaudio.cpp @@ -1389,9 +1389,6 @@ bool PulseBackendFactory::init() #ifdef HAVE_DYNLOAD if(!pulse_handle) { - bool ret{true}; - std::string missing_funcs; - #ifdef _WIN32 #define PALIB "libpulse-0.dll" #elif defined(__APPLE__) && defined(__MACH__) @@ -1406,17 +1403,15 @@ bool PulseBackendFactory::init() return false; } + std::string missing_funcs; #define LOAD_FUNC(x) do { \ - p##x = al::bit_cast(GetSymbol(pulse_handle, #x)); \ - if(!(p##x)) { \ - ret = false; \ - missing_funcs += "\n" #x; \ - } \ + p##x = reinterpret_cast(GetSymbol(pulse_handle, #x)); \ + if(!(p##x)) missing_funcs += "\n" #x; \ } while(0) PULSE_FUNCS(LOAD_FUNC) #undef LOAD_FUNC - if(!ret) + if(!missing_funcs.empty()) { WARN("Missing expected functions:%s\n", missing_funcs.c_str()); CloseLib(pulse_handle); diff --git a/common/dynload.cpp b/common/dynload.cpp index 86c36e00..333a9435 100644 --- a/common/dynload.cpp +++ b/common/dynload.cpp @@ -3,13 +3,12 @@ #include "dynload.h" -#include "albit.h" -#include "strutils.h" - #ifdef _WIN32 #define WIN32_LEAN_AND_MEAN #include +#include "strutils.h" + void *LoadLib(const char *name) { std::wstring wname{utf8_to_wstr(name)}; @@ -18,7 +17,7 @@ void *LoadLib(const char *name) void CloseLib(void *handle) { FreeLibrary(static_cast(handle)); } void *GetSymbol(void *handle, const char *name) -{ return al::bit_cast(GetProcAddress(static_cast(handle), name)); } +{ return reinterpret_cast(GetProcAddress(static_cast(handle), name)); } #elif defined(HAVE_DLFCN_H) diff --git a/core/dbus_wrap.cpp b/core/dbus_wrap.cpp index 08020c9b..05d9fc06 100644 --- a/core/dbus_wrap.cpp +++ b/core/dbus_wrap.cpp @@ -8,7 +8,6 @@ #include #include -#include "albit.h" #include "logging.h" @@ -16,8 +15,15 @@ void PrepareDBus() { const char *libname{"libdbus-1.so.3"}; + dbus_handle = LoadLib(libname); + if(!dbus_handle) + { + WARN("Failed to load %s\n", libname); + return; + } + auto load_func = [](auto &f, const char *name) -> void - { f = al::bit_cast>(GetSymbol(dbus_handle, name)); }; + { f = reinterpret_cast>(GetSymbol(dbus_handle, name)); }; #define LOAD_FUNC(x) do { \ load_func(p##x, #x); \ if(!p##x) \ @@ -29,14 +35,8 @@ void PrepareDBus() } \ } while(0); - dbus_handle = LoadLib(libname); - if(!dbus_handle) - { - WARN("Failed to load %s\n", libname); - return; - } + DBUS_FUNCTIONS(LOAD_FUNC) -DBUS_FUNCTIONS(LOAD_FUNC) #undef LOAD_FUNC } #endif -- cgit v1.2.3