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 /indra/llcommon | |
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.
Diffstat (limited to 'indra/llcommon')
-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 |
14 files changed, 86 insertions, 50 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); } |