From 17b5b82d64686d482e6dcf96ee54fd62234d5f27 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Thu, 15 Sep 2016 18:15:37 -0700 Subject: [PATCH] Fix warnings in libutils headers system/core/include is included in the global include path using -isystem, which hides all warnings. Fix warnings in libutils headers in preparation for moving from -isystem to -I. - Fix implicit cast from int64_t to long in Condition.h. Remove the __LP64__ check and always compare against LONG_MAX before casting. - Fix implicit cast from size_t to ssize_t in KeyedVector.h - Fix -Wshadow-field-in-constructor warnings in Looper.h and RefBase.h - Move destructors for MessageHandler and LooperCallback to Looper.cpp and ReferenceRenamer and VirtualLightRefBase to RefBase.cpp to prevent vtables in every compilation unit. - Declare template variables in Singleton.h - Fix old-style casts in StrongPointer.h and TypeHelpers.h - Use template metaprogramming in TypeHelpers.h to avoid warnings on memmove on non-trivial types. - Add an assignment operator to key_value_pair_t to complete rule-of-three - Use memcpy instead of dereferencing a reinterpret_casted pointer to treat the bits of a float or double as int32_t or int64_t - Escape unicode sequences inside doxygen comments between \code and \endcode - Remove WIN32 ZD definition in Compat.h, %zd works fine with mingw - Fix WIN32 printf warnings in Filemap.cpp - Initialize mNullValue with 0 in LruCache.h, some of the tests use a non-pointer type for TValue. Test: m -j native Bug: 31492149 Change-Id: I385a05a3ca01258e44fe3b37ef77e4aaff547b26 --- include/utils/Compat.h | 5 -- include/utils/Condition.h | 9 +-- include/utils/KeyedVector.h | 2 +- include/utils/Looper.h | 10 ++-- include/utils/LruCache.h | 2 +- include/utils/RefBase.h | 33 ++++++----- include/utils/Singleton.h | 11 +++- include/utils/StrongPointer.h | 4 +- include/utils/TypeHelpers.h | 104 ++++++++++++++++++++-------------- include/utils/Unicode.h | 2 + include/utils/Vector.h | 4 +- libutils/FileMap.cpp | 6 +- libutils/Looper.cpp | 4 ++ libutils/RefBase.cpp | 4 ++ 14 files changed, 115 insertions(+), 85 deletions(-) diff --git a/include/utils/Compat.h b/include/utils/Compat.h index b2ba55ef1..2709e3b32 100644 --- a/include/utils/Compat.h +++ b/include/utils/Compat.h @@ -45,13 +45,8 @@ static inline ssize_t pwrite64(int fd, const void* buf, size_t nbytes, off64_t o #define DEFFILEMODE 0666 #endif /* _WIN32 */ -#if defined(_WIN32) -#define ZD "%ld" -#define ZD_TYPE long -#else #define ZD "%zd" #define ZD_TYPE ssize_t -#endif /* * Needed for cases where something should be constexpr if possible, but not diff --git a/include/utils/Condition.h b/include/utils/Condition.h index 5650598f0..25a53aa23 100644 --- a/include/utils/Condition.h +++ b/include/utils/Condition.h @@ -17,6 +17,7 @@ #ifndef _LIBS_UTILS_CONDITION_H #define _LIBS_UTILS_CONDITION_H +#include #include #include #include @@ -120,7 +121,7 @@ inline status_t Condition::waitRelative(Mutex& mutex, nsecs_t reltime) { // On 32-bit devices, tv_sec is 32-bit, but `reltime` is 64-bit. int64_t reltime_sec = reltime/1000000000; - ts.tv_nsec += reltime%1000000000; + ts.tv_nsec += static_cast(reltime%1000000000); if (reltime_sec < INT64_MAX && ts.tv_nsec >= 1000000000) { ts.tv_nsec -= 1000000000; ++reltime_sec; @@ -133,11 +134,7 @@ inline status_t Condition::waitRelative(Mutex& mutex, nsecs_t reltime) { time_sec += reltime_sec; } -#if defined(__LP64__) - ts.tv_sec = time_sec; -#else - ts.tv_sec = (time_sec > INT32_MAX) ? INT32_MAX : time_sec; -#endif + ts.tv_sec = (time_sec > LONG_MAX) ? LONG_MAX : static_cast(time_sec); return -pthread_cond_timedwait(&mCond, &mutex.mMutex, &ts); } diff --git a/include/utils/KeyedVector.h b/include/utils/KeyedVector.h index 44e9c08d4..e3d19e1e8 100644 --- a/include/utils/KeyedVector.h +++ b/include/utils/KeyedVector.h @@ -181,7 +181,7 @@ template inline ssize_t KeyedVector::replaceValueAt(size_t index, const VALUE& item) { if (index(index); } return BAD_INDEX; } diff --git a/include/utils/Looper.h b/include/utils/Looper.h index da2d5f2b5..a62e67f5f 100644 --- a/include/utils/Looper.h +++ b/include/utils/Looper.h @@ -49,7 +49,7 @@ typedef int (*Looper_callbackFunc)(int fd, int events, void* data); */ struct Message { Message() : what(0) { } - Message(int what) : what(what) { } + Message(int w) : what(w) { } /* The message type. (interpretation is left up to the handler) */ int what; @@ -66,7 +66,7 @@ struct Message { */ class MessageHandler : public virtual RefBase { protected: - virtual ~MessageHandler() { } + virtual ~MessageHandler(); public: /** @@ -97,7 +97,7 @@ private: */ class LooperCallback : public virtual RefBase { protected: - virtual ~LooperCallback() { } + virtual ~LooperCallback(); public: /** @@ -436,8 +436,8 @@ private: struct MessageEnvelope { MessageEnvelope() : uptime(0) { } - MessageEnvelope(nsecs_t uptime, const sp handler, - const Message& message) : uptime(uptime), handler(handler), message(message) { + MessageEnvelope(nsecs_t u, const sp h, + const Message& m) : uptime(u), handler(h), message(m) { } nsecs_t uptime; diff --git a/include/utils/LruCache.h b/include/utils/LruCache.h index f4e225ad9..89dccd613 100644 --- a/include/utils/LruCache.h +++ b/include/utils/LruCache.h @@ -166,7 +166,7 @@ LruCache::LruCache(uint32_t maxCapacity) , mOldest(NULL) , mYoungest(NULL) , mMaxCapacity(maxCapacity) - , mNullValue(NULL) { + , mNullValue(0) { mSet->max_load_factor(1.0); }; diff --git a/include/utils/RefBase.h b/include/utils/RefBase.h index 3c318c4b2..c6466d3f3 100644 --- a/include/utils/RefBase.h +++ b/include/utils/RefBase.h @@ -212,7 +212,7 @@ protected: // subclasses; we have to make it protected to guarantee that it // cannot be called from this base class (and to make strict compilers // happy). - ~ReferenceRenamer() { } + ~ReferenceRenamer(); public: virtual void operator()(size_t i) const = 0; }; @@ -372,7 +372,7 @@ private: // destructor to eliminate the template requirement of LightRefBase class VirtualLightRefBase : public LightRefBase { public: - virtual ~VirtualLightRefBase() {} + virtual ~VirtualLightRefBase(); }; // --------------------------------------------------------------------------- @@ -646,42 +646,42 @@ public: // a template template... template static inline - void move_references(sp* d, sp const* s, size_t n) { + void move_references(sp* dest, sp const* src, size_t n) { class Renamer : public ReferenceRenamer { - sp* d; - sp const* s; + sp* d_; + sp const* s_; virtual void operator()(size_t i) const { // The id are known to be the sp<>'s this pointer - TYPE::renameRefId(d[i].get(), &s[i], &d[i]); + TYPE::renameRefId(d_[i].get(), &s_[i], &d_[i]); } public: - Renamer(sp* d, sp const* s) : d(d), s(s) { } + Renamer(sp* d, sp const* s) : d_(d), s_(s) { } virtual ~Renamer() { } }; - memmove(d, s, n*sizeof(sp)); - TYPE::renameRefs(n, Renamer(d, s)); + memmove(dest, src, n*sizeof(sp)); + TYPE::renameRefs(n, Renamer(dest, src)); } template static inline - void move_references(wp* d, wp const* s, size_t n) { + void move_references(wp* dest, wp const* src, size_t n) { class Renamer : public ReferenceRenamer { - wp* d; - wp const* s; + wp* d_; + wp const* s_; virtual void operator()(size_t i) const { // The id are known to be the wp<>'s this pointer - TYPE::renameRefId(d[i].get_refs(), &s[i], &d[i]); + TYPE::renameRefId(d_[i].get_refs(), &s_[i], &d_[i]); } public: - Renamer(wp* d, wp const* s) : d(d), s(s) { } + Renamer(wp* rd, wp const* rs) : d_(rd), s_(rs) { } virtual ~Renamer() { } }; - memmove(d, s, n*sizeof(wp)); - TYPE::renameRefs(n, Renamer(d, s)); + memmove(dest, src, n*sizeof(wp)); + TYPE::renameRefs(n, Renamer(dest, src)); } }; @@ -712,7 +712,6 @@ void move_backward_type(wp* d, wp const* s, size_t n) { ReferenceMover::move_references(d, s, n); } - }; // namespace android // --------------------------------------------------------------------------- diff --git a/include/utils/Singleton.h b/include/utils/Singleton.h index ffc03cb5d..7cc4c18be 100644 --- a/include/utils/Singleton.h +++ b/include/utils/Singleton.h @@ -19,6 +19,7 @@ #include #include +#include #include #include @@ -45,8 +46,8 @@ public: } protected: - ~Singleton() { }; - Singleton() { }; + ~Singleton() { } + Singleton() { } private: Singleton(const Singleton&); @@ -55,6 +56,12 @@ private: static TYPE* sInstance; }; +template +Mutex Singleton::sLock; + +template +TYPE* Singleton::sInstance; + /* * use ANDROID_SINGLETON_STATIC_INSTANCE(TYPE) in your implementation file * (eg: .cpp) to create the static instance of Singleton<>'s attributes, diff --git a/include/utils/StrongPointer.h b/include/utils/StrongPointer.h index d90b788b1..294e6b6f4 100644 --- a/include/utils/StrongPointer.h +++ b/include/utils/StrongPointer.h @@ -137,7 +137,7 @@ template template sp::sp(U* other) : m_ptr(other) { if (other) - ((T*) other)->incStrong(this); + (static_cast(other))->incStrong(this); } template template @@ -212,7 +212,7 @@ sp& sp::operator =(sp&& other) { template template sp& sp::operator =(U* other) { if (other) - ((T*) other)->incStrong(this); + (static_cast(other))->incStrong(this); if (m_ptr) m_ptr->decStrong(this); m_ptr = other; diff --git a/include/utils/TypeHelpers.h b/include/utils/TypeHelpers.h index 64d25c5db..627579350 100644 --- a/include/utils/TypeHelpers.h +++ b/include/utils/TypeHelpers.h @@ -18,6 +18,8 @@ #define ANDROID_TYPE_HELPERS_H #include +#include + #include #include #include @@ -178,49 +180,61 @@ void splat_type(TYPE* where, const TYPE* what, size_t n) { } } -template inline -void move_forward_type(TYPE* d, const TYPE* s, size_t n = 1) { - if ((traits::has_trivial_dtor && traits::has_trivial_copy) - || traits::has_trivial_move) - { - memmove(d,s,n*sizeof(TYPE)); - } else { - d += n; - s += n; - while (n > 0) { - n--; - --d, --s; - if (!traits::has_trivial_copy) { - new(d) TYPE(*s); - } else { - *d = *s; - } - if (!traits::has_trivial_dtor) { - s->~TYPE(); - } +template +struct use_trivial_move : public std::integral_constant::has_trivial_dtor && traits::has_trivial_copy) + || traits::has_trivial_move +> {}; + +template +typename std::enable_if::value>::type +inline +move_forward_type(TYPE* d, const TYPE* s, size_t n = 1) { + memmove(d, s, n*sizeof(TYPE)); +} + +template +typename std::enable_if::value>::type +inline +move_forward_type(TYPE* d, const TYPE* s, size_t n = 1) { + d += n; + s += n; + while (n > 0) { + n--; + --d, --s; + if (!traits::has_trivial_copy) { + new(d) TYPE(*s); + } else { + *d = *s; + } + if (!traits::has_trivial_dtor) { + s->~TYPE(); } } } -template inline -void move_backward_type(TYPE* d, const TYPE* s, size_t n = 1) { - if ((traits::has_trivial_dtor && traits::has_trivial_copy) - || traits::has_trivial_move) - { - memmove(d,s,n*sizeof(TYPE)); - } else { - while (n > 0) { - n--; - if (!traits::has_trivial_copy) { - new(d) TYPE(*s); - } else { - *d = *s; - } - if (!traits::has_trivial_dtor) { - s->~TYPE(); - } - d++, s++; +template +typename std::enable_if::value>::type +inline +move_backward_type(TYPE* d, const TYPE* s, size_t n = 1) { + memmove(d, s, n*sizeof(TYPE)); +} + +template +typename std::enable_if::value>::type +inline +move_backward_type(TYPE* d, const TYPE* s, size_t n = 1) { + while (n > 0) { + n--; + if (!traits::has_trivial_copy) { + new(d) TYPE(*s); + } else { + *d = *s; } + if (!traits::has_trivial_dtor) { + s->~TYPE(); + } + d++, s++; } } @@ -239,6 +253,11 @@ struct key_value_pair_t { VALUE value; key_value_pair_t() { } key_value_pair_t(const key_value_pair_t& o) : key(o.key), value(o.value) { } + key_value_pair_t& operator=(const key_value_pair_t& o) { + key = o.key; + value = o.value; + return *this; + } key_value_pair_t(const KEY& k, const VALUE& v) : key(k), value(v) { } explicit key_value_pair_t(const KEY& k) : key(k) { } inline bool operator < (const key_value_pair_t& o) const { @@ -275,8 +294,7 @@ typedef uint32_t hash_t; template hash_t hash_type(const TKey& key); -/* Built-in hash code specializations. - * Assumes pointers are 32bit. */ +/* Built-in hash code specializations */ #define ANDROID_INT32_HASH(T) \ template <> inline hash_t hash_type(const T& value) { return hash_t(value); } #define ANDROID_INT64_HASH(T) \ @@ -284,7 +302,11 @@ hash_t hash_type(const TKey& key); return hash_t((value >> 32) ^ value); } #define ANDROID_REINTERPRET_HASH(T, R) \ template <> inline hash_t hash_type(const T& value) { \ - return hash_type(*reinterpret_cast(&value)); } + R newValue; \ + static_assert(sizeof(newValue) == sizeof(value), "size mismatch"); \ + memcpy(&newValue, &value, sizeof(newValue)); \ + return hash_type(newValue); \ + } ANDROID_INT32_HASH(bool) ANDROID_INT32_HASH(int8_t) diff --git a/include/utils/Unicode.h b/include/utils/Unicode.h index a13f34779..666b70f4e 100644 --- a/include/utils/Unicode.h +++ b/include/utils/Unicode.h @@ -60,6 +60,7 @@ ssize_t utf32_to_utf8_length(const char32_t *src, size_t src_len); * Returns the size actually used for storing the string. * dst" is not nul-terminated when dst_len is fully used (like strncpy). * + * \code * Example 1 * "src" == \u3042\u3044 (\xE3\x81\x82\xE3\x81\x84) * "src_len" == 2 @@ -87,6 +88,7 @@ ssize_t utf32_to_utf8_length(const char32_t *src, size_t src_len); * Returned value == 6 * "dst" becomes \xE3\x81\x82\xE3\x81\x84 * (note that "dst" is NOT nul-terminated, like strncpy) + * \endcode */ void utf32_to_utf8(const char32_t* src, size_t src_len, char* dst, size_t dst_len); diff --git a/include/utils/Vector.h b/include/utils/Vector.h index 86800f562..81ac9c7a1 100644 --- a/include/utils/Vector.h +++ b/include/utils/Vector.h @@ -371,12 +371,12 @@ ssize_t Vector::removeItemsAt(size_t index, size_t count) { template inline status_t Vector::sort(Vector::compar_t cmp) { - return VectorImpl::sort((VectorImpl::compar_t)cmp); + return VectorImpl::sort(reinterpret_cast(cmp)); } template inline status_t Vector::sort(Vector::compar_r_t cmp, void* state) { - return VectorImpl::sort((VectorImpl::compar_r_t)cmp, state); + return VectorImpl::sort(reinterpret_cast(cmp), state); } // --------------------------------------------------------------------------- diff --git a/libutils/FileMap.cpp b/libutils/FileMap.cpp index 4f4b8895a..1afa1ecae 100644 --- a/libutils/FileMap.cpp +++ b/libutils/FileMap.cpp @@ -98,7 +98,7 @@ FileMap::~FileMap(void) } #if defined(__MINGW32__) if (mBasePtr && UnmapViewOfFile(mBasePtr) == 0) { - ALOGD("UnmapViewOfFile(%p) failed, error = %" PRId32 "\n", mBasePtr, + ALOGD("UnmapViewOfFile(%p) failed, error = %lu\n", mBasePtr, GetLastError() ); } if (mFileMapping != INVALID_HANDLE_VALUE) { @@ -138,7 +138,7 @@ bool FileMap::create(const char* origFileName, int fd, off64_t offset, size_t le mFileHandle = (HANDLE) _get_osfhandle(fd); mFileMapping = CreateFileMapping( mFileHandle, NULL, protect, 0, 0, NULL); if (mFileMapping == NULL) { - ALOGE("CreateFileMapping(%p, %" PRIx32 ") failed with error %" PRId32 "\n", + ALOGE("CreateFileMapping(%p, %lx) failed with error %lu\n", mFileHandle, protect, GetLastError() ); return false; } @@ -153,7 +153,7 @@ bool FileMap::create(const char* origFileName, int fd, off64_t offset, size_t le (DWORD)(adjOffset), adjLength ); if (mBasePtr == NULL) { - ALOGE("MapViewOfFile(%" PRId64 ", %zu) failed with error %" PRId32 "\n", + ALOGE("MapViewOfFile(%" PRId64 ", %zu) failed with error %lu\n", adjOffset, adjLength, GetLastError() ); CloseHandle(mFileMapping); mFileMapping = INVALID_HANDLE_VALUE; diff --git a/libutils/Looper.cpp b/libutils/Looper.cpp index 952c992f1..adb0f8d47 100644 --- a/libutils/Looper.cpp +++ b/libutils/Looper.cpp @@ -677,4 +677,8 @@ void Looper::Request::initEventItem(struct epoll_event* eventItem) const { eventItem->data.fd = fd; } +MessageHandler::~MessageHandler() { } + +LooperCallback::~LooperCallback() { } + } // namespace android diff --git a/libutils/RefBase.cpp b/libutils/RefBase.cpp index fee9984a7..ba1aaee39 100644 --- a/libutils/RefBase.cpp +++ b/libutils/RefBase.cpp @@ -770,4 +770,8 @@ void RefBase::renameRefId(RefBase* ref, ref->mRefs->renameWeakRefId(old_id, new_id); } +ReferenceRenamer::~ReferenceRenamer() {} + +VirtualLightRefBase::~VirtualLightRefBase() {} + }; // namespace android