From da45a5a7c1c76ab28e21d66b552edc4c9f7cc214 Mon Sep 17 00:00:00 2001 From: serge-sans-paille Date: Tue, 31 Jan 2023 16:44:04 +0000 Subject: [PATCH] Bug 1811934 - Have nsID::ToString() return an managed string instead of a raw pointer r=xpcom-reviewers,nika And use nsID::ToProvidedString(...) method when suitable. This naturally fixes a memory leak in dom/fetch/FetchParent.cpp. Differential Revision: https://phabricator.services.mozilla.com/D167606 --- dom/fetch/FetchParent.cpp | 2 +- js/xpconnect/src/XPCWrappedJS.cpp | 10 +++------- js/xpconnect/src/XPCWrappedJSClass.cpp | 7 ++----- .../extensions/webidl-api/ExtensionAPIRequest.cpp | 2 +- xpcom/base/nsID.cpp | 14 +++----------- xpcom/base/nsID.h | 11 +++++++---- xpcom/components/nsComponentManager.cpp | 12 ++++-------- xpcom/ds/nsVariant.cpp | 7 +------ xpcom/tests/gtest/TestID.cpp | 7 ++----- 9 files changed, 24 insertions(+), 48 deletions(-) diff --git a/dom/fetch/FetchParent.cpp b/dom/fetch/FetchParent.cpp index 3639e064d2ae..5395ceb30296 100644 --- a/dom/fetch/FetchParent.cpp +++ b/dom/fetch/FetchParent.cpp @@ -27,7 +27,7 @@ FetchParent::FetchParentCSPEventListener::FetchParentCSPEventListener( : mActorID(aActorID), mEventTarget(aEventTarget) { MOZ_ASSERT(mEventTarget); FETCH_LOG(("FetchParentCSPEventListener [%p] actor ID: %s", this, - mActorID.ToString())); + mActorID.ToString().get())); } NS_IMETHODIMP FetchParent::FetchParentCSPEventListener::OnCSPViolationEvent( diff --git a/js/xpconnect/src/XPCWrappedJS.cpp b/js/xpconnect/src/XPCWrappedJS.cpp index 2a2f33b344b4..7cb550491e59 100644 --- a/js/xpconnect/src/XPCWrappedJS.cpp +++ b/js/xpconnect/src/XPCWrappedJS.cpp @@ -594,8 +594,7 @@ nsXPCWrappedJS* nsXPCWrappedJS::FindInherited(REFNSIID aIID) { return nullptr; } -nsresult -nsIXPConnectWrappedJS::GetInterfaceIID(nsIID** iid) { +nsresult nsIXPConnectWrappedJS::GetInterfaceIID(nsIID** iid) { MOZ_ASSERT(iid, "bad param"); *iid = AsXPCWrappedJS()->GetIID().Clone(); @@ -662,11 +661,8 @@ nsresult nsXPCWrappedJS::DebugDump(int16_t depth) { IsRootWrapper() ? "ROOT" : "non-root", mJSObj.get())); const char* name = mInfo->Name(); XPC_LOG_ALWAYS(("interface name is %s", name)); - char* iid = mInfo->IID().ToString(); - XPC_LOG_ALWAYS(("IID number is %s", iid ? iid : "invalid")); - if (iid) { - free(iid); - } + auto iid = mInfo->IID().ToString(); + XPC_LOG_ALWAYS(("IID number is %s", iid.get())); XPC_LOG_ALWAYS(("nsXPTInterfaceInfo @ %p", mInfo)); if (!IsRootWrapper()) { diff --git a/js/xpconnect/src/XPCWrappedJSClass.cpp b/js/xpconnect/src/XPCWrappedJSClass.cpp index e4c1fd195be8..0c1627fdc499 100644 --- a/js/xpconnect/src/XPCWrappedJSClass.cpp +++ b/js/xpconnect/src/XPCWrappedJSClass.cpp @@ -1065,11 +1065,8 @@ void nsXPCWrappedJS::DebugDumpInterfaceInfo(const nsXPTInterfaceInfo* aInfo, XPC_LOG_INDENT(); const char* name = aInfo->Name(); XPC_LOG_ALWAYS(("interface name is %s", name)); - char* iid = aInfo->IID().ToString(); - XPC_LOG_ALWAYS(("IID number is %s", iid ? iid : "invalid")); - if (iid) { - free(iid); - } + auto iid = aInfo->IID().ToString(); + XPC_LOG_ALWAYS(("IID number is %s", iid.get())); XPC_LOG_ALWAYS(("InterfaceInfo @ %p", aInfo)); uint16_t methodCount = 0; if (depth) { diff --git a/toolkit/components/extensions/webidl-api/ExtensionAPIRequest.cpp b/toolkit/components/extensions/webidl-api/ExtensionAPIRequest.cpp index bb744ded567a..e5dd4436953d 100644 --- a/toolkit/components/extensions/webidl-api/ExtensionAPIRequest.cpp +++ b/toolkit/components/extensions/webidl-api/ExtensionAPIRequest.cpp @@ -39,7 +39,7 @@ ExtensionServiceWorkerInfo::GetScriptURL(nsAString& aScriptURL) { NS_IMETHODIMP ExtensionServiceWorkerInfo::GetClientInfoId(nsAString& aClientInfoId) { MOZ_ASSERT(NS_IsMainThread()); - aClientInfoId = NS_ConvertUTF8toUTF16(mClientInfo.Id().ToString()); + aClientInfoId = NS_ConvertUTF8toUTF16(mClientInfo.Id().ToString().get()); return NS_OK; } diff --git a/xpcom/base/nsID.cpp b/xpcom/base/nsID.cpp index 58a24abc06f5..31d85c0c4372 100644 --- a/xpcom/base/nsID.cpp +++ b/xpcom/base/nsID.cpp @@ -163,18 +163,10 @@ static const char gIDFormat[] = "{%08x-%04x-%04x-%02x%02x-%02x%02x%02x%02x%02x%02x}"; /* - * Returns an allocated string in {xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx} - * format. The string is allocated with moz_xmalloc and should be freed by - * the caller. + * Returns a managed string in {xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx} + * format. */ - -char* nsID::ToString() const { - char* res = (char*)moz_xmalloc(NSID_LENGTH); - snprintf(res, NSID_LENGTH, gIDFormat, m0, (uint32_t)m1, (uint32_t)m2, - (uint32_t)m3[0], (uint32_t)m3[1], (uint32_t)m3[2], (uint32_t)m3[3], - (uint32_t)m3[4], (uint32_t)m3[5], (uint32_t)m3[6], (uint32_t)m3[7]); - return res; -} +nsIDToCString nsID::ToString() const { return nsIDToCString(*this); } void nsID::ToProvidedString(char (&aDest)[NSID_LENGTH]) const { SprintfLiteral(aDest, gIDFormat, m0, (uint32_t)m1, (uint32_t)m2, diff --git a/xpcom/base/nsID.h b/xpcom/base/nsID.h index f0332d899bed..c7feddd1ccfa 100644 --- a/xpcom/base/nsID.h +++ b/xpcom/base/nsID.h @@ -13,6 +13,10 @@ #define NSID_LENGTH 39 +#ifndef XPCOM_GLUE_AVOID_NSPR +class nsIDToCString; +#endif + /** * A "unique identifier". This is modeled after OSF DCE UUIDs. */ @@ -69,11 +73,10 @@ struct nsID { #ifndef XPCOM_GLUE_AVOID_NSPR /** - * nsID string encoder. Returns an allocated string in - * {xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx} format. Caller should free string. - * YOU SHOULD ONLY USE THIS IF YOU CANNOT USE ToProvidedString() BELOW. + * nsID string encoder. Returns a managed string in + * {xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx} format. */ - char* ToString() const; + nsIDToCString ToString() const; /** * nsID string encoder. Builds a string in diff --git a/xpcom/components/nsComponentManager.cpp b/xpcom/components/nsComponentManager.cpp index 7147fd8333d1..35954b170205 100644 --- a/xpcom/components/nsComponentManager.cpp +++ b/xpcom/components/nsComponentManager.cpp @@ -657,11 +657,9 @@ nsComponentManagerImpl::GetClassObject(const nsCID& aClass, const nsIID& aIID, nsresult rv; if (MOZ_LOG_TEST(nsComponentManagerLog, LogLevel::Debug)) { - char* buf = aClass.ToString(); + char buf[NSID_LENGTH]; + aClass.ToProvidedString(buf); PR_LogPrint("nsComponentManager: GetClassObject(%s)", buf); - if (buf) { - free(buf); - } } MOZ_ASSERT(aResult != nullptr); @@ -765,13 +763,11 @@ nsComponentManagerImpl::CreateInstance(const nsCID& aClass, const nsIID& aIID, } if (MOZ_LOG_TEST(nsComponentManagerLog, LogLevel::Warning)) { - char* buf = aClass.ToString(); + char buf[NSID_LENGTH]; + aClass.ToProvidedString(buf); MOZ_LOG(nsComponentManagerLog, LogLevel::Warning, ("nsComponentManager: CreateInstance(%s) %s", buf, NS_SUCCEEDED(rv) ? "succeeded" : "FAILED")); - if (buf) { - free(buf); - } } return rv; diff --git a/xpcom/ds/nsVariant.cpp b/xpcom/ds/nsVariant.cpp index 3ccfc7fa3165..266182779ca3 100644 --- a/xpcom/ds/nsVariant.cpp +++ b/xpcom/ds/nsVariant.cpp @@ -703,12 +703,7 @@ nsresult nsDiscriminatedUnion::ToString(nsACString& aOutString) const { // nsID has its own text formatter. case nsIDataType::VTYPE_ID: { - char* ptr = u.mIDValue.ToString(); - if (!ptr) { - return NS_ERROR_OUT_OF_MEMORY; - } - aOutString.Assign(ptr); - free(ptr); + aOutString.Assign(u.mIDValue.ToString().get()); return NS_OK; } diff --git a/xpcom/tests/gtest/TestID.cpp b/xpcom/tests/gtest/TestID.cpp index d9a712500bed..65b41a2b265b 100644 --- a/xpcom/tests/gtest/TestID.cpp +++ b/xpcom/tests/gtest/TestID.cpp @@ -27,10 +27,7 @@ TEST(nsID, StringConversion) const char* idstr = ids[i]; ASSERT_TRUE(id.Parse(idstr)); - char* cp = id.ToString(); - ASSERT_NE(cp, nullptr); - ASSERT_STREQ(cp, ids[4 * (i / 4) + 3]); - - free(cp); + auto cp = id.ToString(); + ASSERT_STREQ(cp.get(), ids[4 * (i / 4) + 3]); } }