diff options
author | Nat Goodspeed <nat@lindenlab.com> | 2022-11-12 18:59:21 -1000 |
---|---|---|
committer | Nat Goodspeed <nat@lindenlab.com> | 2022-11-12 18:59:21 -1000 |
commit | 4349cb6165e983ff6bdd45ad1b82bb98bfc0436f (patch) | |
tree | eeb39af41808a4846a16a558118a4b929abe71bb | |
parent | cd997f21c272987e954f5890ed14fcc327eb734e (diff) |
DRTVWR-575: Address review comments on Xcode 14.1 type tweaks.
Introduce LLSD template constructors and assignment operators to disambiguate
construction or assignment from any integer type to Integer, likewise any
floating point type to Real. Use new narrow() function to validate
conversions.
For LLSD method parameters converted from LLSD::Integer to size_t, where the
method previously checked for a negative argument, make it now check for
size_t converted from negative: in other words, more than S32_MAX. The risk of
having a parameter forced from negative to unsigned exceeds the risk of a
valid length or index over that max.
In lltracerecording.cpp's PeriodicRecording, now that mCurPeriod and
mNumRecordedPeriods are size_t instead of S32, defend against subtracting 1
from 0.
Use narrow() to validate newly-introduced narrowing conversions.
Make llclamp() return the type of the raw input value, even if the types of
the boundary values differ.
std::ostream::tellp() no longer returns a value we can directly report as a
number. Cast to U64.
-rw-r--r-- | indra/llcommon/llbase64.cpp | 4 | ||||
-rw-r--r-- | indra/llcommon/lldefs.h | 7 | ||||
-rw-r--r-- | indra/llcommon/llleap.cpp | 6 | ||||
-rw-r--r-- | indra/llcommon/llsd.cpp | 22 | ||||
-rw-r--r-- | indra/llcommon/llsd.h | 50 | ||||
-rw-r--r-- | indra/llcommon/llsdserialize.cpp | 2 | ||||
-rw-r--r-- | indra/llcommon/llsdserialize_xml.cpp | 4 | ||||
-rw-r--r-- | indra/llcommon/llsys.cpp | 2 | ||||
-rw-r--r-- | indra/llcommon/llsys_objc.mm | 12 | ||||
-rw-r--r-- | indra/llcommon/lltrace.cpp | 2 | ||||
-rw-r--r-- | indra/llcommon/lltraceaccumulators.cpp | 2 | ||||
-rw-r--r-- | indra/llcommon/lltracerecording.cpp | 7 | ||||
-rw-r--r-- | indra/llcommon/llunittype.h | 8 | ||||
-rw-r--r-- | indra/llcommon/stdtypes.h | 8 | ||||
-rw-r--r-- | indra/newview/lltexturecache.cpp | 2 | ||||
-rw-r--r-- | indra/newview/lltexturecache.h | 2 | ||||
-rw-r--r-- | indra/newview/lltexturefetch.cpp | 6 | ||||
-rw-r--r-- | indra/newview/lltexturefetch.h | 4 |
18 files changed, 93 insertions, 57 deletions
diff --git a/indra/llcommon/llbase64.cpp b/indra/llcommon/llbase64.cpp index 85f22969b8..bb85fe32a3 100644 --- a/indra/llcommon/llbase64.cpp +++ b/indra/llcommon/llbase64.cpp @@ -42,7 +42,7 @@ std::string LLBase64::encode(const U8* input, size_t input_size) && input_size > 0) { // Yes, it returns int. - int b64_buffer_length = apr_base64_encode_len(int(input_size)); + int b64_buffer_length = apr_base64_encode_len(narrow(input_size)); char* b64_buffer = new char[b64_buffer_length]; // This is faster than apr_base64_encode() if you know @@ -52,7 +52,7 @@ std::string LLBase64::encode(const U8* input, size_t input_size) b64_buffer_length = apr_base64_encode_binary( b64_buffer, input, - int(input_size)); + narrow(input_size)); output.assign(b64_buffer); delete[] b64_buffer; } diff --git a/indra/llcommon/lldefs.h b/indra/llcommon/lldefs.h index d9b0cbf71b..a567fd7c12 100644 --- a/indra/llcommon/lldefs.h +++ b/indra/llcommon/lldefs.h @@ -212,16 +212,15 @@ inline auto llmin(T1 d1, T2 d2, T3 d3, T4 d4) } template <typename A, typename MIN, typename MAX> -inline typename std::common_type<A, MIN, MAX>::type -llclamp(A a, MIN minval, MAX maxval) +inline A llclamp(A a, MIN minval, MAX maxval) { if ( a < minval ) { - return minval; + return static_cast<A>(minval); } else if ( a > maxval ) { - return maxval; + return static_cast<A>(maxval); } return a; } diff --git a/indra/llcommon/llleap.cpp b/indra/llcommon/llleap.cpp index 2704f8b6de..c87c0758fe 100644 --- a/indra/llcommon/llleap.cpp +++ b/indra/llcommon/llleap.cpp @@ -231,7 +231,8 @@ public: } |*==========================================================================*/ - LL_DEBUGS("EventHost") << "Sending: " << buffer.tellp() << ':'; + LL_DEBUGS("EventHost") << "Sending: " + << static_cast<U64>(buffer.tellp()) << ':'; std::string::size_type truncate(80); if (buffer.tellp() <= truncate) { @@ -244,7 +245,8 @@ public: LL_CONT << LL_ENDL; LLProcess::WritePipe& childin(mChild->getWritePipe(LLProcess::STDIN)); - childin.get_ostream() << buffer.tellp() << ':' << buffer.str() << std::flush; + childin.get_ostream() << static_cast<U64>(buffer.tellp()) + << ':' << buffer.str() << std::flush; return false; } diff --git a/indra/llcommon/llsd.cpp b/indra/llcommon/llsd.cpp index 8772178b05..a645e624f8 100644 --- a/indra/llcommon/llsd.cpp +++ b/indra/llcommon/llsd.cpp @@ -36,6 +36,18 @@ #include "llsdserialize.h" #include "stringize.h" +#include <limits> + +// Defend against a caller forcibly passing a negative number into an unsigned +// size_t index param +inline +bool was_negative(size_t i) +{ + return (i > std::numeric_limits<int>::max()); +} +#define NEGATIVE_EXIT(i) if (was_negative(i)) return +#define NEGATIVE_RETURN(i, result) NEGATIVE_EXIT(i) (result) + #ifndef LL_RELEASE_FOR_DOWNLOAD #define NAME_UNNAMED_NAMESPACE #endif @@ -555,6 +567,7 @@ namespace LLSD ImplArray::get(size_t i) const { + NEGATIVE_RETURN(i, LLSD()); DataVector::size_type index = i; return (index < mData.size()) ? mData[index] : LLSD(); @@ -562,6 +575,7 @@ namespace void ImplArray::set(size_t i, const LLSD& v) { + NEGATIVE_EXIT(i); DataVector::size_type index = i; if (index >= mData.size()) @@ -574,6 +588,7 @@ namespace void ImplArray::insert(size_t i, const LLSD& v) { + NEGATIVE_EXIT(i); DataVector::size_type index = i; if (index >= mData.size()) // tbd - sanity check limit for index ? @@ -592,6 +607,7 @@ namespace void ImplArray::erase(size_t i) { + NEGATIVE_EXIT(i); DataVector::size_type index = i; if (index < mData.size()) @@ -602,7 +618,7 @@ namespace LLSD& ImplArray::ref(size_t i) { - DataVector::size_type index = i; + DataVector::size_type index = was_negative(i)? 0 : i; if (index >= mData.size()) { @@ -614,6 +630,7 @@ namespace const LLSD& ImplArray::ref(size_t i) const { + NEGATIVE_RETURN(i, undef()); DataVector::size_type index = i; if (index >= mData.size()) @@ -833,9 +850,6 @@ LLSD::LLSD(const Date& v) : impl(0) { ALLOC_LLSD_OBJECT; assign(v); } LLSD::LLSD(const URI& v) : impl(0) { ALLOC_LLSD_OBJECT; assign(v); } LLSD::LLSD(const Binary& v) : impl(0) { ALLOC_LLSD_OBJECT; assign(v); } -// Convenience Constructors -LLSD::LLSD(F32 v) : impl(0) { ALLOC_LLSD_OBJECT; assign((Real)v); } - // Scalar Assignment void LLSD::assign(Boolean v) { safe(impl).assign(impl, v); } void LLSD::assign(Integer v) { safe(impl).assign(impl, v); } diff --git a/indra/llcommon/llsd.h b/indra/llcommon/llsd.h index f034470da7..c1406cf73f 100644 --- a/indra/llcommon/llsd.h +++ b/indra/llcommon/llsd.h @@ -30,6 +30,7 @@ #include <map> #include <string> #include <vector> +#include <type_traits> #include "stdtypes.h" @@ -192,7 +193,17 @@ public: /** @name Convenience Constructors */ //@{ - LLSD(F32); // F32 -> Real + // support construction from size_t et al. + template <typename VALUE, + typename std::enable_if<std::is_integral<VALUE>::value && + ! std::is_same<VALUE, Boolean>::value, + bool>::type = true> + LLSD(VALUE v): LLSD(Integer(narrow(v))) {} + // support construction from F32 et al. + template <typename VALUE, + typename std::enable_if<std::is_floating_point<VALUE>::value, + bool>::type = true> + LLSD(VALUE v): LLSD(Real(narrow(v))) {} //@} /** @name Scalar Assignment */ @@ -205,15 +216,21 @@ public: void assign(const Date&); void assign(const URI&); void assign(const Binary&); - - LLSD& operator=(Boolean v) { assign(v); return *this; } - LLSD& operator=(Integer v) { assign(v); return *this; } - LLSD& operator=(Real v) { assign(v); return *this; } - LLSD& operator=(const String& v) { assign(v); return *this; } - LLSD& operator=(const UUID& v) { assign(v); return *this; } - LLSD& operator=(const Date& v) { assign(v); return *this; } - LLSD& operator=(const URI& v) { assign(v); return *this; } - LLSD& operator=(const Binary& v) { assign(v); return *this; } + + // support assignment from size_t et al. + template <typename VALUE, + typename std::enable_if<std::is_integral<VALUE>::value && + ! std::is_same<VALUE, Boolean>::value, + bool>::type = true> + void assign(VALUE v) { assign(Integer(narrow(v))); } + // support assignment from F32 et al. + template <typename VALUE, + typename std::enable_if<std::is_floating_point<VALUE>::value, + bool>::type = true> + void assign(VALUE v) { assign(Real(narrow(v))); } + + template <typename VALUE> + LLSD& operator=(VALUE v) { assign(v); return *this; } //@} /** @@ -275,7 +292,6 @@ public: //@{ LLSD(const char*); void assign(const char*); - LLSD& operator=(const char* v) { assign(v); return *this; } //@} /** @name Map Values */ @@ -317,9 +333,15 @@ public: // accept size_t so we can index relative to size() const LLSD& operator[](size_t) const; LLSD& operator[](size_t); - // overload to disambiguate [0], [1] et al. - const LLSD& operator[](Integer i) const { return (*this)[size_t(i)]; } - LLSD& operator[](Integer i) { return (*this)[size_t(i)]; } + // template overloads to support int literals, U32 et al. + template <typename IDX, + typename std::enable_if<std::is_convertible<IDX, size_t>::value, + bool>::type = true> + const LLSD& operator[](IDX i) const { return (*this)[size_t(i)]; } + template <typename IDX, + typename std::enable_if<std::is_convertible<IDX, size_t>::value, + bool>::type = true> + LLSD& operator[](IDX i) { return (*this)[size_t(i)]; } //@} /** @name Iterators */ diff --git a/indra/llcommon/llsdserialize.cpp b/indra/llcommon/llsdserialize.cpp index 666da717b2..b720e6adb6 100644 --- a/indra/llcommon/llsdserialize.cpp +++ b/indra/llcommon/llsdserialize.cpp @@ -2110,7 +2110,7 @@ std::string zip_llsd(LLSD& data) U8 out[CHUNK]; - strm.avail_in = uint32_t(source.size()); + strm.avail_in = narrow(source.size()); strm.next_in = (U8*) source.data(); U8* output = NULL; diff --git a/indra/llcommon/llsdserialize_xml.cpp b/indra/llcommon/llsdserialize_xml.cpp index f47c06a2dc..b8b827135d 100644 --- a/indra/llcommon/llsdserialize_xml.cpp +++ b/indra/llcommon/llsdserialize_xml.cpp @@ -196,12 +196,12 @@ S32 LLSDXMLFormatter::format_impl(const LLSD& data, std::ostream& ostr, // *FIX: memory inefficient. // *TODO: convert to use LLBase64 ostr << pre << "<binary encoding=\"base64\">"; - int b64_buffer_length = apr_base64_encode_len(int(buffer.size())); + int b64_buffer_length = apr_base64_encode_len(narrow(buffer.size())); char* b64_buffer = new char[b64_buffer_length]; b64_buffer_length = apr_base64_encode_binary( b64_buffer, &buffer[0], - int(buffer.size())); + narrow(buffer.size())); ostr.write(b64_buffer, b64_buffer_length - 1); delete[] b64_buffer; ostr << "</binary>" << post; diff --git a/indra/llcommon/llsys.cpp b/indra/llcommon/llsys.cpp index 1951ca6843..7a545843c0 100644 --- a/indra/llcommon/llsys.cpp +++ b/indra/llcommon/llsys.cpp @@ -909,7 +909,7 @@ void LLMemoryInfo::stream(std::ostream& s) const // Now stream stats BOOST_FOREACH(const MapEntry& pair, inMap(mStatsMap)) { - s << pfx << std::setw(int(key_width+1)) << (pair.first + ':') << ' '; + s << pfx << std::setw(narrow(key_width+1)) << (pair.first + ':') << ' '; LLSD value(pair.second); if (value.isInteger()) s << std::setw(12) << value.asInteger(); diff --git a/indra/llcommon/llsys_objc.mm b/indra/llcommon/llsys_objc.mm index 9359503a19..81032658d7 100644 --- a/indra/llcommon/llsys_objc.mm +++ b/indra/llcommon/llsys_objc.mm @@ -27,19 +27,19 @@ #import "llsys_objc.h" #import <AppKit/AppKit.h> -static NSInteger intAtStringIndex(NSArray *array, int index) +static int intAtStringIndex(NSArray *array, int index) { - return [(NSString *)[array objectAtIndex:index] integerValue]; + return int([(NSString *)[array objectAtIndex:index] integerValue]); } -bool LLGetDarwinOSInfo(NSInteger &major, NSInteger &minor, NSInteger &patch) +bool LLGetDarwinOSInfo(int &major, int &minor, int &patch) { if (NSAppKitVersionNumber > NSAppKitVersionNumber10_8) { NSOperatingSystemVersion osVersion = [[NSProcessInfo processInfo] operatingSystemVersion]; - major = osVersion.majorVersion; - minor = osVersion.minorVersion; - patch = osVersion.patchVersion; + major = int(osVersion.majorVersion); + minor = int(osVersion.minorVersion); + patch = int(osVersion.patchVersion); } else { diff --git a/indra/llcommon/lltrace.cpp b/indra/llcommon/lltrace.cpp index 818eb0457b..7ad50d1288 100644 --- a/indra/llcommon/lltrace.cpp +++ b/indra/llcommon/lltrace.cpp @@ -65,7 +65,7 @@ void TimeBlockTreeNode::setParent( BlockTimerStatHandle* parent ) llassert_always(parent != mBlock); llassert_always(parent != NULL); - TimeBlockTreeNode* parent_tree_node = get_thread_recorder()->getTimeBlockTreeNode(S32(parent->getIndex())); + TimeBlockTreeNode* parent_tree_node = get_thread_recorder()->getTimeBlockTreeNode(narrow(parent->getIndex())); if (!parent_tree_node) return; if (mParent) diff --git a/indra/llcommon/lltraceaccumulators.cpp b/indra/llcommon/lltraceaccumulators.cpp index 84c1ccd9fe..7c38cdb7cd 100644 --- a/indra/llcommon/lltraceaccumulators.cpp +++ b/indra/llcommon/lltraceaccumulators.cpp @@ -98,7 +98,7 @@ void AccumulatorBufferGroup::makeCurrent() // update stacktimer parent pointers for (size_t i = 0, end_i = mStackTimers.size(); i < end_i; i++) { - TimeBlockTreeNode* tree_node = thread_recorder->getTimeBlockTreeNode(S32(i)); + TimeBlockTreeNode* tree_node = thread_recorder->getTimeBlockTreeNode(narrow(i)); if (tree_node) { timer_accumulator_buffer[i].mParent = tree_node->mParent; diff --git a/indra/llcommon/lltracerecording.cpp b/indra/llcommon/lltracerecording.cpp index 1d31d7b9d0..8414b234e0 100644 --- a/indra/llcommon/lltracerecording.cpp +++ b/indra/llcommon/lltracerecording.cpp @@ -606,7 +606,8 @@ void PeriodicRecording::nextPeriod() mCurPeriod = (mCurPeriod + 1) % mRecordingPeriods.size(); old_recording.splitTo(getCurRecording()); - mNumRecordedPeriods = llmin(mRecordingPeriods.size() - 1, mNumRecordedPeriods + 1); + mNumRecordedPeriods = mRecordingPeriods.empty()? 0 : + llmin(mRecordingPeriods.size() - 1, mNumRecordedPeriods + 1); } void PeriodicRecording::appendRecording(Recording& recording) @@ -652,8 +653,8 @@ void PeriodicRecording::appendPeriodicRecording( PeriodicRecording& other ) mRecordingPeriods.push_back(other.mRecordingPeriods[other_current_recording_index]); } - mCurPeriod = mRecordingPeriods.size() - 1; - mNumRecordedPeriods = mRecordingPeriods.size() - 1; + mCurPeriod = mRecordingPeriods.empty()? 0 : mRecordingPeriods.size() - 1; + mNumRecordedPeriods = mCurPeriod; } else { diff --git a/indra/llcommon/llunittype.h b/indra/llcommon/llunittype.h index e283e8f79a..81f244e422 100644 --- a/indra/llcommon/llunittype.h +++ b/indra/llcommon/llunittype.h @@ -83,10 +83,8 @@ struct LLUnit typedef void is_unit_t; // value initialization - // allow for convertible type - template <typename T=storage_t> - LL_FORCE_INLINE explicit LLUnit(T value = T()) - : mValue(storage_t(value)) + LL_FORCE_INLINE explicit LLUnit(storage_t value = storage_t()) + : mValue(value) {} @@ -126,7 +124,7 @@ struct LLUnit // unit initialization and conversion template<typename OTHER_STORAGE_TYPE, typename OTHER_UNITS> LL_FORCE_INLINE LLUnit(LLUnit<OTHER_STORAGE_TYPE, OTHER_UNITS> other) - : mValue(storage_t(convert(other).mValue)) + : mValue(convert(other).mValue) {} LL_FORCE_INLINE storage_t value() const diff --git a/indra/llcommon/stdtypes.h b/indra/llcommon/stdtypes.h index a030017e3b..da8512169c 100644 --- a/indra/llcommon/stdtypes.h +++ b/indra/llcommon/stdtypes.h @@ -179,10 +179,10 @@ public: { // The reason we skip the // assert(value >= std::numeric_limits<TO>::lowest()); - // in the overload below is that to perform the above comparison, the - // compiler promotes the signed lowest() to the unsigned FROM type, - // making it hugely positive -- so a reasonable 'value' will always - // fail the assert(). + // like the overload below is that to perform the above comparison, + // the compiler promotes the signed lowest() to the unsigned FROM + // type, making it hugely positive -- so a reasonable 'value' will + // always fail the assert(). assert(mValue <= std::numeric_limits<TO>::max()); return static_cast<TO>(mValue); } diff --git a/indra/newview/lltexturecache.cpp b/indra/newview/lltexturecache.cpp index 9403e73b87..cabc3a3517 100644 --- a/indra/newview/lltexturecache.cpp +++ b/indra/newview/lltexturecache.cpp @@ -859,7 +859,7 @@ LLTextureCache::~LLTextureCache() ////////////////////////////////////////////////////////////////////////////// //virtual -S32 LLTextureCache::update(F32 max_time_ms) +size_t LLTextureCache::update(F32 max_time_ms) { static LLFrameTimer timer ; static const F32 MAX_TIME_INTERVAL = 300.f ; //seconds. diff --git a/indra/newview/lltexturecache.h b/indra/newview/lltexturecache.h index e1c752b58e..e6437ef5ea 100644 --- a/indra/newview/lltexturecache.h +++ b/indra/newview/lltexturecache.h @@ -113,7 +113,7 @@ public: LLTextureCache(bool threaded); ~LLTextureCache(); - /*virtual*/ S32 update(F32 max_time_ms); + /*virtual*/ size_t update(F32 max_time_ms); void purgeCache(ELLPath location, bool remove_dir = true); void setReadOnly(BOOL read_only) ; diff --git a/indra/newview/lltexturefetch.cpp b/indra/newview/lltexturefetch.cpp index 0edaf40c66..8cd6ccba85 100644 --- a/indra/newview/lltexturefetch.cpp +++ b/indra/newview/lltexturefetch.cpp @@ -3104,9 +3104,9 @@ bool LLTextureFetch::updateRequestPriority(const LLUUID& id, F32 priority) // Threads: T* //virtual -S32 LLTextureFetch::getPending() +size_t LLTextureFetch::getPending() { - S32 res; + size_t res; lockData(); // +Ct { LLMutexLock lock(&mQueueMutex); // +Mfq @@ -3181,7 +3181,7 @@ void LLTextureFetch::commonUpdate() // Threads: Tmain //virtual -S32 LLTextureFetch::update(F32 max_time_ms) +size_t LLTextureFetch::update(F32 max_time_ms) { static LLCachedControl<F32> band_width(gSavedSettings,"ThrottleBandwidthKBPS", 3000.0); diff --git a/indra/newview/lltexturefetch.h b/indra/newview/lltexturefetch.h index bf6732963f..e2d2aa365c 100644 --- a/indra/newview/lltexturefetch.h +++ b/indra/newview/lltexturefetch.h @@ -65,7 +65,7 @@ public: class TFRequest; // Threads: Tmain - /*virtual*/ S32 update(F32 max_time_ms); + /*virtual*/ size_t update(F32 max_time_ms); // called in the main thread after the TextureCacheThread shuts down. // Threads: Tmain @@ -137,7 +137,7 @@ public: U32 getTotalNumHTTPRequests(); // Threads: T* - S32 getPending(); + size_t getPending(); // Threads: T* void lockQueue() { mQueueMutex.lock(); } |