diff options
author | Nicky Dasmijn <nicky.dasmijn@gmail.com> | 2020-07-18 13:55:55 +0200 |
---|---|---|
committer | Andrey Lihatskiy <alihatskiy@productengine.com> | 2020-07-22 06:48:15 +0300 |
commit | acaa2723308a3cd5b90c0f337520478f67910804 (patch) | |
tree | 23fea9e1a9c3d1928afb001c02bdc0c8ebb9b4d9 | |
parent | c16a364377fef29454c958087121de103fa9de41 (diff) |
LLExtStat had been a S32, this wasn't right, as some of the constants lead to integer overflow: const LLExtStat LL_EXSTAT_RES_RESULT = 2L<<30; const LLExtStat LL_EXSTAT_VFS_RESULT = 3L<<30; This shifts into the sign bit and clang gets (rightfully) upset about this.
LLExtStatus needs to be at least of type U32 to remedy this problem, but
while at it it makes sense to turn it into what it is: An enum. Turning
it into a class enum has the added benefit we get type safety for mostly
free.
Which incidentally turned up a problem right away:
A call to removeAndCallbackPendingDownloads had status and extstatus
reversed and thus was wrong.
-rw-r--r-- | indra/llmessage/llassetstorage.cpp | 30 | ||||
-rw-r--r-- | indra/llmessage/llextendedstatus.h | 51 | ||||
-rw-r--r-- | indra/llmessage/lltransfertargetvfile.cpp | 2 | ||||
-rw-r--r-- | indra/llmessage/llxfer.cpp | 2 | ||||
-rw-r--r-- | indra/llmessage/llxfer_mem.cpp | 2 | ||||
-rw-r--r-- | indra/newview/llsettingsvo.cpp | 2 | ||||
-rw-r--r-- | indra/newview/llviewerassetstorage.cpp | 24 |
7 files changed, 56 insertions, 57 deletions
diff --git a/indra/llmessage/llassetstorage.cpp b/indra/llmessage/llassetstorage.cpp index 18b2b124e1..94be56086b 100644 --- a/indra/llmessage/llassetstorage.cpp +++ b/indra/llmessage/llassetstorage.cpp @@ -426,11 +426,11 @@ void LLAssetStorage::_cleanupRequests(BOOL all, S32 error) LLAssetRequest* tmp = *curiter; if (tmp->mUpCallback) { - tmp->mUpCallback(tmp->getUUID(), tmp->mUserData, error, LL_EXSTAT_NONE); + tmp->mUpCallback(tmp->getUUID(), tmp->mUserData, error, LLExtStat::LL_EXSTAT_NONE); } if (tmp->mDownCallback) { - tmp->mDownCallback(mVFS, tmp->getUUID(), tmp->getType(), tmp->mUserData, error, LL_EXSTAT_NONE); + tmp->mDownCallback(mVFS, tmp->getUUID(), tmp->getType(), tmp->mUserData, error, LLExtStat::LL_EXSTAT_NONE); } if (tmp->mInfoCallback) { @@ -465,7 +465,7 @@ bool LLAssetStorage::findInStaticVFSAndInvokeCallback(const LLUUID& uuid, LLAsse // we've already got the file if (callback) { - callback(mStaticVFS, uuid, type, user_data, LL_ERR_NOERR, LL_EXSTAT_VFS_CACHED); + callback(mStaticVFS, uuid, type, user_data, LL_ERR_NOERR, LLExtStat::LL_EXSTAT_VFS_CACHED); } return true; } @@ -506,7 +506,7 @@ void LLAssetStorage::getAssetData(const LLUUID uuid, if (callback) { add(sFailedDownloadCount, 1); - callback(mVFS, uuid, type, user_data, LL_ERR_ASSET_REQUEST_FAILED, LL_EXSTAT_NONE); + callback(mVFS, uuid, type, user_data, LL_ERR_ASSET_REQUEST_FAILED, LLExtStat::LL_EXSTAT_NONE); } return; } @@ -517,7 +517,7 @@ void LLAssetStorage::getAssetData(const LLUUID uuid, if (callback) { add(sFailedDownloadCount, 1); - callback(mVFS, uuid, type, user_data, LL_ERR_ASSET_REQUEST_NOT_IN_DATABASE, LL_EXSTAT_NULL_UUID); + callback(mVFS, uuid, type, user_data, LL_ERR_ASSET_REQUEST_NOT_IN_DATABASE, LLExtStat::LL_EXSTAT_NULL_UUID); } return; } @@ -540,7 +540,7 @@ void LLAssetStorage::getAssetData(const LLUUID uuid, // unless there's a weird error if (callback) { - callback(mVFS, uuid, type, user_data, LL_ERR_NOERR, LL_EXSTAT_VFS_CACHED); + callback(mVFS, uuid, type, user_data, LL_ERR_NOERR, LLExtStat::LL_EXSTAT_VFS_CACHED); } LL_DEBUGS("AssetStorage") << "ASSET_TRACE asset " << uuid << " found in VFS" << LL_ENDL; @@ -694,7 +694,7 @@ void LLAssetStorage::downloadCompleteCallback( } } - removeAndCallbackPendingDownloads(file_id, file_type, callback_id, callback_type, ext_status, result); + removeAndCallbackPendingDownloads(file_id, file_type, callback_id, callback_type, result, ext_status); } void LLAssetStorage::getEstateAsset( @@ -719,7 +719,7 @@ void LLAssetStorage::getEstateAsset( if (callback) { add(sFailedDownloadCount, 1); - callback(mVFS, asset_id, atype, user_data, LL_ERR_ASSET_REQUEST_NOT_IN_DATABASE, LL_EXSTAT_NULL_UUID); + callback(mVFS, asset_id, atype, user_data, LL_ERR_ASSET_REQUEST_NOT_IN_DATABASE, LLExtStat::LL_EXSTAT_NULL_UUID); } return; } @@ -741,7 +741,7 @@ void LLAssetStorage::getEstateAsset( // unless there's a weird error if (callback) { - callback(mVFS, asset_id, atype, user_data, LL_ERR_NOERR, LL_EXSTAT_VFS_CACHED); + callback(mVFS, asset_id, atype, user_data, LL_ERR_NOERR, LLExtStat::LL_EXSTAT_VFS_CACHED); } } else @@ -792,7 +792,7 @@ void LLAssetStorage::getEstateAsset( if (callback) { add(sFailedDownloadCount, 1); - callback(mVFS, asset_id, atype, user_data, LL_ERR_CIRCUIT_GONE, LL_EXSTAT_NO_UPSTREAM); + callback(mVFS, asset_id, atype, user_data, LL_ERR_CIRCUIT_GONE, LLExtStat::LL_EXSTAT_NO_UPSTREAM); } } } @@ -885,7 +885,7 @@ void LLAssetStorage::getInvItemAsset( // unless there's a weird error if (callback) { - callback(mVFS, asset_id, atype, user_data, LL_ERR_NOERR, LL_EXSTAT_VFS_CACHED); + callback(mVFS, asset_id, atype, user_data, LL_ERR_NOERR, LLExtStat::LL_EXSTAT_VFS_CACHED); } } else @@ -936,7 +936,7 @@ void LLAssetStorage::getInvItemAsset( if (callback) { add(sFailedDownloadCount, 1); - callback(mVFS, asset_id, atype, user_data, LL_ERR_CIRCUIT_GONE, LL_EXSTAT_NO_UPSTREAM); + callback(mVFS, asset_id, atype, user_data, LL_ERR_CIRCUIT_GONE, LLExtStat::LL_EXSTAT_NO_UPSTREAM); } } } @@ -1034,7 +1034,7 @@ void LLAssetStorage::processUploadComplete(LLMessageSystem *msg, void **user_dat msg->getBOOLFast(_PREHASH_AssetBlock, _PREHASH_Success, success); asset_type = (LLAssetType::EType)asset_type_s8; - this_ptr->_callUploadCallbacks(uuid, asset_type, success, LL_EXSTAT_NONE); + this_ptr->_callUploadCallbacks(uuid, asset_type, success, LLExtStat::LL_EXSTAT_NONE); } void LLAssetStorage::_callUploadCallbacks(const LLUUID &uuid, LLAssetType::EType asset_type, BOOL success, LLExtStat ext_status ) @@ -1288,12 +1288,12 @@ bool LLAssetStorage::deletePendingRequestImpl(LLAssetStorage::request_list_t* re // Run callbacks. if (req->mUpCallback) { - req->mUpCallback(req->getUUID(), req->mUserData, error, LL_EXSTAT_REQUEST_DROPPED); + req->mUpCallback(req->getUUID(), req->mUserData, error, LLExtStat::LL_EXSTAT_REQUEST_DROPPED); } if (req->mDownCallback) { add(sFailedDownloadCount, 1); - req->mDownCallback(mVFS, req->getUUID(), req->getType(), req->mUserData, error, LL_EXSTAT_REQUEST_DROPPED); + req->mDownCallback(mVFS, req->getUUID(), req->getType(), req->mUserData, error, LLExtStat::LL_EXSTAT_REQUEST_DROPPED); } if (req->mInfoCallback) { diff --git a/indra/llmessage/llextendedstatus.h b/indra/llmessage/llextendedstatus.h index 8ce173d1ff..8bb8165419 100644 --- a/indra/llmessage/llextendedstatus.h +++ b/indra/llmessage/llextendedstatus.h @@ -28,40 +28,39 @@ #ifndef LL_LLEXTENDEDSTATUS_H #define LL_LLEXTENDEDSTATUS_H +enum class LLExtStat: uint32_t +{ + // Status provider groups - Top bits indicate which status type it is + // Zero is common status code (next section) + LL_EXSTAT_CURL_RESULT = 1UL<<30, // serviced by curl - use 1L if we really implement the below + LL_EXSTAT_RES_RESULT = 2UL<<30, // serviced by resident copy + LL_EXSTAT_VFS_RESULT = 3UL<<30, // serviced by vfs -typedef S32 LLExtStat; + // Common Status Codes + // + LL_EXSTAT_NONE = 0x00000, // No extra info here - sorry! + LL_EXSTAT_NULL_UUID = 0x10001, // null asset ID + LL_EXSTAT_NO_UPSTREAM = 0x10002, // attempt to upload without a valid upstream method/provider + LL_EXSTAT_REQUEST_DROPPED = 0x10003, // request was dropped unserviced + LL_EXSTAT_NONEXISTENT_FILE = 0x10004, // trying to upload a file that doesn't exist + LL_EXSTAT_BLOCKED_FILE = 0x10005, // trying to upload a file that we can't open -// Status provider groups - Top bits indicate which status type it is -// Zero is common status code (next section) -const LLExtStat LL_EXSTAT_CURL_RESULT = 1L<<30; // serviced by curl - use 1L if we really implement the below -const LLExtStat LL_EXSTAT_RES_RESULT = 2L<<30; // serviced by resident copy -const LLExtStat LL_EXSTAT_VFS_RESULT = 3L<<30; // serviced by vfs + // curl status codes: + // + // Mask off LL_EXSTAT_CURL_RESULT for original result and + // see: libraries/include/curl/curl.h -// Common Status Codes -// -const LLExtStat LL_EXSTAT_NONE = 0x00000; // No extra info here - sorry! -const LLExtStat LL_EXSTAT_NULL_UUID = 0x10001; // null asset ID -const LLExtStat LL_EXSTAT_NO_UPSTREAM = 0x10002; // attempt to upload without a valid upstream method/provider -const LLExtStat LL_EXSTAT_REQUEST_DROPPED = 0x10003; // request was dropped unserviced -const LLExtStat LL_EXSTAT_NONEXISTENT_FILE = 0x10004; // trying to upload a file that doesn't exist -const LLExtStat LL_EXSTAT_BLOCKED_FILE = 0x10005; // trying to upload a file that we can't open + // Memory-Resident status codes: + // None at present -// curl status codes: -// -// Mask off LL_EXSTAT_CURL_RESULT for original result and -// see: libraries/include/curl/curl.h - -// Memory-Resident status codes: -// None at present - - -// VFS status codes: -const LLExtStat LL_EXSTAT_VFS_CACHED = LL_EXSTAT_VFS_RESULT | 0x0001; -const LLExtStat LL_EXSTAT_VFS_CORRUPT = LL_EXSTAT_VFS_RESULT | 0x0002; + // VFS status codes: + LL_EXSTAT_VFS_CACHED = LL_EXSTAT_VFS_RESULT | 0x0001, + LL_EXSTAT_VFS_CORRUPT = LL_EXSTAT_VFS_RESULT | 0x0002, +}; #endif // LL_LLEXTENDEDSTATUS_H diff --git a/indra/llmessage/lltransfertargetvfile.cpp b/indra/llmessage/lltransfertargetvfile.cpp index a572c68a7f..6fc971cc69 100644 --- a/indra/llmessage/lltransfertargetvfile.cpp +++ b/indra/llmessage/lltransfertargetvfile.cpp @@ -227,7 +227,7 @@ void LLTransferTargetVFile::completionCallback(const LLTSCode status) mParams.getAssetID(), mParams.getAssetType(), mParams.mRequestDatap, - LL_EXSTAT_NONE); + LLExtStat::LL_EXSTAT_NONE); } delete mParams.mRequestDatap; mParams.mRequestDatap = NULL; diff --git a/indra/llmessage/llxfer.cpp b/indra/llmessage/llxfer.cpp index 32e0e2cc3b..fcd77b93a8 100644 --- a/indra/llmessage/llxfer.cpp +++ b/indra/llmessage/llxfer.cpp @@ -319,7 +319,7 @@ S32 LLXfer::processEOF() if (mCallback) { - mCallback(mCallbackDataHandle,mCallbackResult,LL_EXSTAT_NONE); + mCallback(mCallbackDataHandle,mCallbackResult, LLExtStat::LL_EXSTAT_NONE); } return(retval); diff --git a/indra/llmessage/llxfer_mem.cpp b/indra/llmessage/llxfer_mem.cpp index 78a3e4f558..7bd78b7c75 100644 --- a/indra/llmessage/llxfer_mem.cpp +++ b/indra/llmessage/llxfer_mem.cpp @@ -112,7 +112,7 @@ S32 LLXfer_Mem::processEOF() if (mCallback) { - mCallback((void *)mBuffer,mBufferLength,mCallbackDataHandle,mCallbackResult,LL_EXSTAT_NONE); + mCallback((void *)mBuffer,mBufferLength,mCallbackDataHandle,mCallbackResult, LLExtStat::LL_EXSTAT_NONE); } return(retval); diff --git a/indra/newview/llsettingsvo.cpp b/indra/newview/llsettingsvo.cpp index c72a0706cd..97b5b2a57d 100644 --- a/indra/newview/llsettingsvo.cpp +++ b/indra/newview/llsettingsvo.cpp @@ -329,7 +329,7 @@ void LLSettingsVOBase::onAssetDownloadComplete(LLVFS *vfs, const LLUUID &asset_i } else { - LL_WARNS("SETTINGS") << "Error retrieving asset " << asset_id << ". Status code=" << status << "(" << LLAssetStorage::getErrorString(status) << ") ext_status=" << ext_status << LL_ENDL; + LL_WARNS("SETTINGS") << "Error retrieving asset " << asset_id << ". Status code=" << status << "(" << LLAssetStorage::getErrorString(status) << ") ext_status=" << (U32)ext_status << LL_ENDL; } if (callback) callback(asset_id, settings, status, ext_status); diff --git a/indra/newview/llviewerassetstorage.cpp b/indra/newview/llviewerassetstorage.cpp index e0b64403ef..26bea20689 100644 --- a/indra/newview/llviewerassetstorage.cpp +++ b/indra/newview/llviewerassetstorage.cpp @@ -168,7 +168,7 @@ void LLViewerAssetStorage::storeAssetData( delete req; if (callback) { - callback(asset_id, user_data, LL_ERR_ASSET_REQUEST_FAILED, LL_EXSTAT_VFS_CORRUPT); + callback(asset_id, user_data, LL_ERR_ASSET_REQUEST_FAILED, LLExtStat::LL_EXSTAT_VFS_CORRUPT); } return; } @@ -209,7 +209,7 @@ void LLViewerAssetStorage::storeAssetData( if (callback) { - callback(asset_id, user_data, LL_ERR_ASSET_REQUEST_NONEXISTENT_FILE, LL_EXSTAT_VFS_CORRUPT); + callback(asset_id, user_data, LL_ERR_ASSET_REQUEST_NONEXISTENT_FILE, LLExtStat::LL_EXSTAT_VFS_CORRUPT); } return; } @@ -236,7 +236,7 @@ void LLViewerAssetStorage::storeAssetData( reportMetric( asset_id, asset_type, LLStringUtil::null, LLUUID::null, 0, MR_ZERO_SIZE, __FILE__, __LINE__, "The file didn't exist or was zero length (VFS - can't tell which)" ); if (callback) { - callback(asset_id, user_data, LL_ERR_ASSET_REQUEST_NONEXISTENT_FILE, LL_EXSTAT_NONEXISTENT_FILE); + callback(asset_id, user_data, LL_ERR_ASSET_REQUEST_NONEXISTENT_FILE, LLExtStat::LL_EXSTAT_NONEXISTENT_FILE); } } } @@ -247,7 +247,7 @@ void LLViewerAssetStorage::storeAssetData( reportMetric( asset_id, asset_type, LLStringUtil::null, LLUUID::null, 0, MR_NO_UPSTREAM, __FILE__, __LINE__, "No upstream provider" ); if (callback) { - callback(asset_id, user_data, LL_ERR_CIRCUIT_GONE, LL_EXSTAT_NO_UPSTREAM); + callback(asset_id, user_data, LL_ERR_CIRCUIT_GONE, LLExtStat::LL_EXSTAT_NO_UPSTREAM); } } } @@ -333,7 +333,7 @@ void LLViewerAssetStorage::storeAssetData( } if (callback) { - callback(asset_id, user_data, LL_ERR_CANNOT_OPEN_FILE, LL_EXSTAT_BLOCKED_FILE); + callback(asset_id, user_data, LL_ERR_CANNOT_OPEN_FILE, LLExtStat::LL_EXSTAT_BLOCKED_FILE); } } } @@ -444,13 +444,13 @@ void LLViewerAssetStorage::assetRequestCoro( mCountStarted++; S32 result_code = LL_ERR_NOERR; - LLExtStat ext_status = LL_EXSTAT_NONE; + LLExtStat ext_status = LLExtStat::LL_EXSTAT_NONE; if (!gAgent.getRegion()) { LL_WARNS_ONCE("ViewerAsset") << "Asset request fails: no region set" << LL_ENDL; result_code = LL_ERR_ASSET_REQUEST_FAILED; - ext_status = LL_EXSTAT_NONE; + ext_status = LLExtStat::LL_EXSTAT_NONE; removeAndCallbackPendingDownloads(uuid, atype, uuid, atype, result_code, ext_status); return; } @@ -475,7 +475,7 @@ void LLViewerAssetStorage::assetRequestCoro( { LL_WARNS_ONCE("ViewerAsset") << "asset request fails: caps received but no viewer asset cap found" << LL_ENDL; result_code = LL_ERR_ASSET_REQUEST_FAILED; - ext_status = LL_EXSTAT_NONE; + ext_status = LLExtStat::LL_EXSTAT_NONE; removeAndCallbackPendingDownloads(uuid, atype, uuid, atype, result_code, ext_status); return; } @@ -504,7 +504,7 @@ void LLViewerAssetStorage::assetRequestCoro( { LL_DEBUGS("ViewerAsset") << "request failed, status " << status.toTerseString() << LL_ENDL; result_code = LL_ERR_ASSET_REQUEST_FAILED; - ext_status = LL_EXSTAT_NONE; + ext_status = LLExtStat::LL_EXSTAT_NONE; } else { @@ -530,13 +530,13 @@ void LLViewerAssetStorage::assetRequestCoro( // TODO asset-http: handle error LL_WARNS("ViewerAsset") << "Failure in vf.write()" << LL_ENDL; result_code = LL_ERR_ASSET_REQUEST_FAILED; - ext_status = LL_EXSTAT_VFS_CORRUPT; + ext_status = LLExtStat::LL_EXSTAT_VFS_CORRUPT; } else if (!vf.rename(uuid, atype)) { LL_WARNS("ViewerAsset") << "rename failed" << LL_ENDL; result_code = LL_ERR_ASSET_REQUEST_FAILED; - ext_status = LL_EXSTAT_VFS_CORRUPT; + ext_status = LLExtStat::LL_EXSTAT_VFS_CORRUPT; } else { @@ -548,7 +548,7 @@ void LLViewerAssetStorage::assetRequestCoro( // TODO asset-http: handle invalid size case LL_WARNS("ViewerAsset") << "bad size" << LL_ENDL; result_code = LL_ERR_ASSET_REQUEST_FAILED; - ext_status = LL_EXSTAT_NONE; + ext_status = LLExtStat::LL_EXSTAT_NONE; } } |