Bug 413548 - null-arg checks in nsMsgThreadedDBView.cpp, nsMsgSendLater.cpp, nsSmtpUrl.cpp, nsImapIncomingServer.cpp, nsPop3IncomingServer.cpp. r=Standard8
authoraceman <acelists@atlas.sk>
Tue, 21 Feb 2012 12:31:53 +0000
changeset 10880 4487f9fbfe2dacd8cd222b2160b6f59861890a5e
parent 10879 1b2d1c269c890ee7cb11d3865502c6757ebe29b6
child 10881 ef4fd81847414ad3cda327c58eeaddad45323015
push id463
push userbugzilla@standard8.plus.com
push dateTue, 24 Apr 2012 17:34:51 +0000
treeherdercomm-beta@e53588e8f7b0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersStandard8
bugs413548
Bug 413548 - null-arg checks in nsMsgThreadedDBView.cpp, nsMsgSendLater.cpp, nsSmtpUrl.cpp, nsImapIncomingServer.cpp, nsPop3IncomingServer.cpp. r=Standard8
mailnews/base/src/nsMsgThreadedDBView.cpp
mailnews/compose/src/nsMsgSendLater.cpp
mailnews/compose/src/nsSmtpUrl.cpp
mailnews/imap/src/nsImapIncomingServer.cpp
mailnews/local/src/nsPop3IncomingServer.cpp
--- a/mailnews/base/src/nsMsgThreadedDBView.cpp
+++ b/mailnews/base/src/nsMsgThreadedDBView.cpp
@@ -586,130 +586,126 @@ nsresult nsMsgThreadedDBView::InitSort(n
   return NS_OK;
 }
 
 nsresult nsMsgThreadedDBView::OnNewHeader(nsIMsgDBHdr *newHdr, nsMsgKey aParentKey, bool ensureListed)
 {
   if (m_viewFlags & nsMsgViewFlagsType::kGroupBySort)
     return nsMsgGroupView::OnNewHeader(newHdr, aParentKey, ensureListed);
 
-  nsresult rv = NS_OK;
+  NS_ENSURE_TRUE(newHdr, NS_MSG_MESSAGE_NOT_FOUND);
+
   nsMsgKey newKey;
   newHdr->GetMessageKey(&newKey);
 
   // views can override this behaviour, which is to append to view.
   // This is the mail behaviour, but threaded views want
   // to insert in order...
-  if (newHdr)
-  {
-    PRUint32 msgFlags;
-    newHdr->GetFlags(&msgFlags);
-    if ((m_viewFlags & nsMsgViewFlagsType::kUnreadOnly) && !ensureListed && (msgFlags & nsMsgMessageFlags::Read))
-      return NS_OK;
-    // Currently, we only add the header in a threaded view if it's a thread.
-    // We used to check if this was the first header in the thread, but that's
-    // a bit harder in the unreadOnly view. But we'll catch it below.
+  PRUint32 msgFlags;
+  newHdr->GetFlags(&msgFlags);
+  if ((m_viewFlags & nsMsgViewFlagsType::kUnreadOnly) && !ensureListed &&
+      (msgFlags & nsMsgMessageFlags::Read))
+    return NS_OK;
+  // Currently, we only add the header in a threaded view if it's a thread.
+  // We used to check if this was the first header in the thread, but that's
+  // a bit harder in the unreadOnly view. But we'll catch it below.
+
+  // if not threaded display just add it to the view.
+  if (!(m_viewFlags & nsMsgViewFlagsType::kThreadedDisplay))
+    return AddHdr(newHdr);
+
+  // need to find the thread we added this to so we can change the hasnew flag
+  // added message to existing thread, but not to view
+  // Fix flags on thread header.
+  PRInt32 threadCount;
+  PRUint32 threadFlags;
+  bool moveThread = false;
+  nsMsgViewIndex threadIndex = ThreadIndexOfMsg(newKey, nsMsgViewIndex_None, &threadCount, &threadFlags);
+  bool threadRootIsDisplayed = false;
 
-    // if not threaded display just add it to the view.
-    if (!(m_viewFlags & nsMsgViewFlagsType::kThreadedDisplay))
-      rv = AddHdr(newHdr);
-    else // need to find the thread we added this to so we can change the hasnew flag
-         // added message to existing thread, but not to view
-    {    // Fix flags on thread header.
-      PRInt32 threadCount;
-      PRUint32 threadFlags;
-      bool moveThread = false;
-      nsMsgViewIndex threadIndex = ThreadIndexOfMsg(newKey, nsMsgViewIndex_None, &threadCount, &threadFlags);
-      bool threadRootIsDisplayed = false;
+  nsCOMPtr <nsIMsgThread> threadHdr;
+  m_db->GetThreadContainingMsgHdr(newHdr, getter_AddRefs(threadHdr));
+  if (threadHdr && m_sortType == nsMsgViewSortType::byDate)
+  {
+    PRUint32 newestMsgInThread = 0, msgDate = 0;
+    threadHdr->GetNewestMsgDate(&newestMsgInThread);
+    newHdr->GetDateInSeconds(&msgDate);
+    moveThread = (msgDate == newestMsgInThread);
+  }
 
-      nsCOMPtr <nsIMsgThread> threadHdr;
-      m_db->GetThreadContainingMsgHdr(newHdr, getter_AddRefs(threadHdr));
-      if (threadHdr && m_sortType == nsMsgViewSortType::byDate)
-      {
-        PRUint32 newestMsgInThread = 0, msgDate = 0;
-        threadHdr->GetNewestMsgDate(&newestMsgInThread);
-        newHdr->GetDateInSeconds(&msgDate);
-        moveThread = (msgDate == newestMsgInThread);
-      }
-      if (threadIndex != nsMsgViewIndex_None)
+  if (threadIndex != nsMsgViewIndex_None)
+  {
+    threadRootIsDisplayed = (m_currentlyDisplayedViewIndex == threadIndex);
+    PRUint32 flags = m_flags[threadIndex];
+    if (!(flags & MSG_VIEW_FLAG_HASCHILDREN))
+    {
+      flags |= MSG_VIEW_FLAG_HASCHILDREN | MSG_VIEW_FLAG_ISTHREAD;
+      if (!(m_viewFlags & nsMsgViewFlagsType::kUnreadOnly))
+        flags |= nsMsgMessageFlags::Elided;
+      m_flags[threadIndex] = flags;
+    }
+
+    if (!(flags & nsMsgMessageFlags::Elided))
+    { // thread is expanded
+      // insert child into thread
+      // levels of other hdrs may have changed!
+      PRUint32 newFlags = msgFlags;
+      PRInt32 level = 0;
+      nsMsgViewIndex insertIndex = threadIndex;
+      if (aParentKey == nsMsgKey_None)
       {
-        threadRootIsDisplayed = (m_currentlyDisplayedViewIndex == threadIndex);
-        PRUint32 flags = m_flags[threadIndex];
-        if (!(flags & MSG_VIEW_FLAG_HASCHILDREN))
-        {
-          flags |= MSG_VIEW_FLAG_HASCHILDREN | MSG_VIEW_FLAG_ISTHREAD;
-          if (!(m_viewFlags & nsMsgViewFlagsType::kUnreadOnly))
-            flags |= nsMsgMessageFlags::Elided;
-          m_flags[threadIndex] = flags;
-        }
-        if (!(flags & nsMsgMessageFlags::Elided)) // thread is expanded
-        {  // insert child into thread
-          // levels of other hdrs may have changed!
-          PRUint32 newFlags = msgFlags;
-          PRInt32 level = 0;
-          nsMsgViewIndex insertIndex = threadIndex;
-          if (aParentKey == nsMsgKey_None)
-          {
-            newFlags |= MSG_VIEW_FLAG_ISTHREAD | MSG_VIEW_FLAG_HASCHILDREN;
-          }
-          else
-          {
-            nsMsgViewIndex parentIndex = FindParentInThread(aParentKey, threadIndex);
-            level = m_levels[parentIndex] + 1;
-            insertIndex = GetInsertInfoForNewHdr(newHdr, parentIndex, level);
-          }
-          InsertMsgHdrAt(insertIndex, newHdr, newKey, newFlags, level);
-          // the call to NoteChange() has to happen after we add the key
-          // as NoteChange() will call RowCountChanged() which will call our GetRowCount()
-          NoteChange(insertIndex, 1, nsMsgViewNotificationCode::insertOrDelete);
+        newFlags |= MSG_VIEW_FLAG_ISTHREAD | MSG_VIEW_FLAG_HASCHILDREN;
+      }
+      else
+      {
+        nsMsgViewIndex parentIndex = FindParentInThread(aParentKey, threadIndex);
+        level = m_levels[parentIndex] + 1;
+        insertIndex = GetInsertInfoForNewHdr(newHdr, parentIndex, level);
+      }
+      InsertMsgHdrAt(insertIndex, newHdr, newKey, newFlags, level);
+      // the call to NoteChange() has to happen after we add the key
+      // as NoteChange() will call RowCountChanged() which will call our GetRowCount()
+      NoteChange(insertIndex, 1, nsMsgViewNotificationCode::insertOrDelete);
 
-          if (aParentKey == nsMsgKey_None)
-          {
-            // this header is the new king! try collapsing the existing thread,
-            // removing it, installing this header as king, and expanding it.
-            CollapseByIndex(threadIndex, nsnull);
-            // call base class, so child won't get promoted.
-            // nsMsgDBView::RemoveByIndex(threadIndex);
-            ExpandByIndex(threadIndex, nsnull);
-          }
-        }
-        else if (aParentKey == nsMsgKey_None)
-        {
-          // if we have a collapsed thread which just got a new
-          // top of thread, change the keys array.
-          m_keys[threadIndex] = newKey;
-        }
-
-        // If this message is new, the thread is collapsed, it is the
-        // root and it was displayed, expand it so that the user does
-        // not find that their message has magically turned into a summary.
-        if (msgFlags & nsMsgMessageFlags::New &&
-            m_flags[threadIndex] & nsMsgMessageFlags::Elided &&
-            threadRootIsDisplayed)
-          ExpandByIndex(threadIndex, nsnull);
-
-        if (moveThread)
-          MoveThreadAt(threadIndex);
-        else
-        // note change, to update the parent thread's unread and total counts
-          NoteChange(threadIndex, 1, nsMsgViewNotificationCode::changed);
-
-      }
-      else // adding msg to thread that's not in view.
+      if (aParentKey == nsMsgKey_None)
       {
-        if (threadHdr)
-        {
-          AddMsgToThreadNotInView(threadHdr, newHdr, ensureListed);
-        }
+        // this header is the new king! try collapsing the existing thread,
+        // removing it, installing this header as king, and expanding it.
+        CollapseByIndex(threadIndex, nsnull);
+        // call base class, so child won't get promoted.
+        // nsMsgDBView::RemoveByIndex(threadIndex);
+        ExpandByIndex(threadIndex, nsnull);
       }
     }
+    else if (aParentKey == nsMsgKey_None)
+    {
+      // if we have a collapsed thread which just got a new
+      // top of thread, change the keys array.
+      m_keys[threadIndex] = newKey;
+    }
+
+    // If this message is new, the thread is collapsed, it is the
+    // root and it was displayed, expand it so that the user does
+    // not find that their message has magically turned into a summary.
+    if (msgFlags & nsMsgMessageFlags::New &&
+        m_flags[threadIndex] & nsMsgMessageFlags::Elided &&
+        threadRootIsDisplayed)
+      ExpandByIndex(threadIndex, nsnull);
+
+    if (moveThread)
+      MoveThreadAt(threadIndex);
+    else
+      // note change, to update the parent thread's unread and total counts
+      NoteChange(threadIndex, 1, nsMsgViewNotificationCode::changed);
   }
-  else
-    rv = NS_MSG_MESSAGE_NOT_FOUND;
-  return rv;
+  else if (threadHdr)
+    // adding msg to thread that's not in view.
+    AddMsgToThreadNotInView(threadHdr, newHdr, ensureListed);
+
+  return NS_OK;
 }
 
 
 NS_IMETHODIMP nsMsgThreadedDBView::OnParentChanged (nsMsgKey aKeyChanged, nsMsgKey oldParent, nsMsgKey newParent, nsIDBChangeListener *aInstigator)
 {
   // we need to adjust the level of the hdr whose parent changed, and invalidate that row,
   // iff we're in threaded mode.
   if (false && m_viewFlags & nsMsgViewFlagsType::kThreadedDisplay)
--- a/mailnews/compose/src/nsMsgSendLater.cpp
+++ b/mailnews/compose/src/nsMsgSendLater.cpp
@@ -350,52 +350,54 @@ nsMsgSendLater::BuildNewBuffer(const cha
   *totalBufSize = aCount + leftoverSize;
   return NS_OK;
 }
 
 // Got data?
 NS_IMETHODIMP
 nsMsgSendLater::OnDataAvailable(nsIRequest *request, nsISupports *ctxt, nsIInputStream *inStr, PRUint32 sourceOffset, PRUint32 count)
 {
+  NS_ENSURE_ARG_POINTER(inStr);
+
   // This is a little bit tricky since we have to chop random 
   // buffers into lines and deliver the lines...plus keeping the
   // leftovers for next time...some fun, eh?
   //
   nsresult    rv = NS_OK;
-  char        *startBuf; 
+  char        *startBuf;
   char        *endBuf;
   char        *lineEnd;
   char        *newbuf = nsnull;
-  PRUint32    size;  
+  PRUint32    size;
 
   PRUint32    aCount = count;
   char        *aBuf = (char *)PR_Malloc(aCount + 1);
 
   inStr->Read(aBuf, count, &aCount);
 
-  // First, create a new work buffer that will 
+  // First, create a new work buffer that will
   if (NS_FAILED(BuildNewBuffer(aBuf, aCount, &size))) // no leftovers...
   {
     startBuf = (char *)aBuf;
     endBuf = (char *)(aBuf + aCount - 1);
   }
   else  // yum, leftovers...new buffer created...sitting in mLeftoverBuffer
   {
     newbuf = mLeftoverBuffer;
-    startBuf = newbuf; 
+    startBuf = newbuf;
     endBuf = startBuf + size - 1;
-    mLeftoverBuffer = nsnull; // null out this 
+    mLeftoverBuffer = nsnull; // null out this
   }
 
   while (startBuf <= endBuf)
   {
     lineEnd = FindEOL(startBuf, endBuf);
     if (!lineEnd)
     {
-      rv = RebufferLeftovers(startBuf, (endBuf - startBuf) + 1);           
+      rv = RebufferLeftovers(startBuf, (endBuf - startBuf) + 1);
       break;
     }
 
     rv = DeliverQueuedLine(startBuf, (lineEnd - startBuf) + 1);
     if (NS_FAILED(rv))
       break;
 
     startBuf = lineEnd+1;
--- a/mailnews/compose/src/nsSmtpUrl.cpp
+++ b/mailnews/compose/src/nsSmtpUrl.cpp
@@ -423,23 +423,21 @@ nsresult nsMailtoUrl::CleanupMailtoState
     m_bodyPart = "";
     m_bccPart = "";
     m_followUpToPart = "";
     m_fromPart = "";
     m_htmlPart = "";
     m_organizationPart = "";
     m_replyToPart = "";
     m_priorityPart = "";
-	return NS_OK;
+    return NS_OK;
 }
 
 nsresult nsMailtoUrl::ParseUrl()
 {
-  nsresult rv = NS_OK;
-
   // we can get the path from the simple url.....
   nsCString escapedPath;
   m_baseURL->GetPath(escapedPath);
 
   PRInt32 startOfSearchPart = escapedPath.FindChar('?');
   if (startOfSearchPart >= 0)
   {
     // now parse out the search field...
@@ -454,17 +452,17 @@ nsresult nsMailtoUrl::ParseUrl()
       ParseMailtoUrl(searchPart.BeginWriting());
     }
   }
   else if (!escapedPath.IsEmpty())
   {
     MsgUnescapeString(escapedPath, 0, m_toPart);
   }
 
-  return rv;
+  return NS_OK;
 }
 
 NS_IMETHODIMP
 nsMailtoUrl::GetMessageContents(nsACString &aToPart, nsACString &aCcPart,
                                 nsACString &aBccPart, nsACString &aSubjectPart,
                                 nsACString &aBodyPart, nsACString &aHtmlPart,
                                 nsACString &aReferencePart,
                                 nsACString &aNewsgroupPart,
@@ -660,30 +658,29 @@ NS_IMETHODIMP nsMailtoUrl::Equals(nsIURI
 }
 
 NS_IMETHODIMP nsMailtoUrl::Clone(nsIURI **_retval)
 {
   NS_ENSURE_ARG_POINTER(_retval);
 
   nsRefPtr<nsMailtoUrl> clone = new nsMailtoUrl();
 
-  if (!clone)
-    return NS_ERROR_OUT_OF_MEMORY;
+  NS_ENSURE_TRUE(clone, NS_ERROR_OUT_OF_MEMORY);
 
   nsresult rv = m_baseURL->Clone(getter_AddRefs(clone->m_baseURL));
   NS_ENSURE_SUCCESS(rv, rv);
 
   clone->ParseUrl();
   *_retval = clone.forget().get();
   return NS_OK;
-}	
+}
 
-NS_IMETHODIMP nsMailtoUrl::Resolve(const nsACString &relativePath, nsACString &result) 
+NS_IMETHODIMP nsMailtoUrl::Resolve(const nsACString &relativePath, nsACString &result)
 {
-	return m_baseURL->Resolve(relativePath, result);
+  return m_baseURL->Resolve(relativePath, result);
 }
 
 NS_IMETHODIMP nsMailtoUrl::SetRef(const nsACString &aRef)
 {
   return m_baseURL->SetRef(aRef);
 }
 
 NS_IMETHODIMP
@@ -808,47 +805,45 @@ nsSmtpUrl::SetDsnEnvid(const nsACString 
 NS_IMETHODIMP
 nsSmtpUrl::GetDsnEnvid(nsACString &aDsnEnvid)
 {
     aDsnEnvid = m_dsnEnvid;
     return NS_OK;
 }
 
 NS_IMETHODIMP
-nsSmtpUrl::GetSenderIdentity(nsIMsgIdentity * *aSenderIdentity)
+nsSmtpUrl::GetSenderIdentity(nsIMsgIdentity **aSenderIdentity)
 {
-  NS_ENSURE_ARG_POINTER(aSenderIdentity); 
-  
+  NS_ENSURE_ARG_POINTER(aSenderIdentity);
   *aSenderIdentity = m_senderIdentity;
   NS_ADDREF(*aSenderIdentity);
   return NS_OK;
 }
 
-NS_IMETHODIMP 
-nsSmtpUrl::SetSenderIdentity(nsIMsgIdentity * aSenderIdentity) 
+NS_IMETHODIMP
+nsSmtpUrl::SetSenderIdentity(nsIMsgIdentity *aSenderIdentity)
 {
   NS_ENSURE_ARG_POINTER(aSenderIdentity);
-  
   m_senderIdentity = aSenderIdentity;
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsSmtpUrl::SetPrompt(nsIPrompt *aNetPrompt)
 {
     NS_ENSURE_ARG_POINTER(aNetPrompt);
     m_netPrompt = aNetPrompt;
     return NS_OK;
 }
 
 NS_IMETHODIMP
 nsSmtpUrl::GetPrompt(nsIPrompt **aNetPrompt)
 {
     NS_ENSURE_ARG_POINTER(aNetPrompt);
-    if (!m_netPrompt) return NS_ERROR_NULL_POINTER;
+    NS_ENSURE_TRUE(m_netPrompt, NS_ERROR_NULL_POINTER);
     *aNetPrompt = m_netPrompt;
     NS_ADDREF(*aNetPrompt);
     return NS_OK;
 }
 
 NS_IMETHODIMP
 nsSmtpUrl::SetAuthPrompt(nsIAuthPrompt *aNetAuthPrompt)
 {
@@ -856,17 +851,17 @@ nsSmtpUrl::SetAuthPrompt(nsIAuthPrompt *
     m_netAuthPrompt = aNetAuthPrompt;
     return NS_OK;
 }
 
 NS_IMETHODIMP
 nsSmtpUrl::GetAuthPrompt(nsIAuthPrompt **aNetAuthPrompt)
 {
     NS_ENSURE_ARG_POINTER(aNetAuthPrompt);
-    if (!m_netAuthPrompt) return NS_ERROR_NULL_POINTER;
+    NS_ENSURE_TRUE(m_netAuthPrompt, NS_ERROR_NULL_POINTER);
     *aNetAuthPrompt = m_netAuthPrompt;
     NS_ADDREF(*aNetAuthPrompt);
     return NS_OK;
 }
 
 NS_IMETHODIMP
 nsSmtpUrl::SetNotificationCallbacks(nsIInterfaceRequestor* aCallbacks)
 {
@@ -874,17 +869,17 @@ nsSmtpUrl::SetNotificationCallbacks(nsII
     m_callbacks = aCallbacks;
     return NS_OK;
 }
 
 NS_IMETHODIMP
 nsSmtpUrl::GetNotificationCallbacks(nsIInterfaceRequestor** aCallbacks)
 {
     NS_ENSURE_ARG_POINTER(aCallbacks);
-    if (!m_callbacks) return NS_ERROR_NULL_POINTER;
+    NS_ENSURE_TRUE(m_callbacks, NS_ERROR_NULL_POINTER);
     *aCallbacks = m_callbacks;
     NS_ADDREF(*aCallbacks);
     return NS_OK;
 }
 
 NS_IMETHODIMP
 nsSmtpUrl::SetSmtpServer(nsISmtpServer * aSmtpServer)
 {
@@ -892,13 +887,13 @@ nsSmtpUrl::SetSmtpServer(nsISmtpServer *
     m_smtpServer = aSmtpServer;
     return NS_OK;
 }
 
 NS_IMETHODIMP
 nsSmtpUrl::GetSmtpServer(nsISmtpServer ** aSmtpServer)
 {
     NS_ENSURE_ARG_POINTER(aSmtpServer);
-    if (!m_smtpServer) return NS_ERROR_NULL_POINTER;
+    NS_ENSURE_TRUE(m_smtpServer, NS_ERROR_NULL_POINTER);
     *aSmtpServer = m_smtpServer;
     NS_ADDREF(*aSmtpServer);
     return NS_OK;
 }
--- a/mailnews/imap/src/nsImapIncomingServer.cpp
+++ b/mailnews/imap/src/nsImapIncomingServer.cpp
@@ -718,16 +718,18 @@ nsImapIncomingServer::ConnectionTimeOut(
   }
   return retVal;
 }
 
 nsresult
 nsImapIncomingServer::GetImapConnection(nsIImapUrl * aImapUrl,
                                         nsIImapProtocol ** aImapConnection)
 {
+  NS_ENSURE_ARG_POINTER(aImapUrl);
+
   nsresult rv = NS_OK;
   bool canRunUrlImmediately = false;
   bool canRunButBusy = false;
   nsCOMPtr<nsIImapProtocol> connection;
   nsCOMPtr<nsIImapProtocol> freeConnection;
   bool isBusy = false;
   bool isInboxConnection = false;
 
@@ -2300,16 +2302,18 @@ NS_IMETHODIMP nsImapIncomingServer::GetM
 {
   manageMailAccountUrl = m_manageMailAccountUrl;
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsImapIncomingServer::StartPopulatingWithUri(nsIMsgWindow *aMsgWindow, bool aForceToServer /*ignored*/, const char *uri)
 {
+  NS_ENSURE_ARG_POINTER (uri);
+
   nsresult rv;
   mDoingSubscribeDialog = true;
 
   rv = EnsureInner();
   NS_ENSURE_SUCCESS(rv,rv);
   rv = mInner->StartPopulatingWithUri(aMsgWindow, aForceToServer, uri);
   NS_ENSURE_SUCCESS(rv,rv);
 
--- a/mailnews/local/src/nsPop3IncomingServer.cpp
+++ b/mailnews/local/src/nsPop3IncomingServer.cpp
@@ -644,33 +644,31 @@ NS_IMETHODIMP nsPop3IncomingServer::GetR
 {
   NS_ENSURE_ARG_POINTER(aProtocol);
   NS_IF_ADDREF(*aProtocol = m_runningProtocol);
   return NS_OK;
 }
 
 NS_IMETHODIMP nsPop3IncomingServer::AddUidlToMark(const char *aUidl, PRInt32 aMark)
 {
-  Pop3UidlEntry *uidlEntry;
-  nsresult rv = NS_ERROR_OUT_OF_MEMORY;
+  NS_ENSURE_ARG_POINTER(aUidl);
+
+  Pop3UidlEntry *uidlEntry = PR_NEWZAP(Pop3UidlEntry);
+  NS_ENSURE_TRUE(uidlEntry, NS_ERROR_OUT_OF_MEMORY);
 
-  uidlEntry = PR_NEWZAP(Pop3UidlEntry);
-  if (uidlEntry)
-  {
-    uidlEntry->uidl = strdup(aUidl);
-    if (uidlEntry->uidl)
-    {
-      uidlEntry->status = (aMark == POP3_DELETE) ? DELETE_CHAR :
-      (aMark == POP3_FETCH_BODY) ? FETCH_BODY : KEEP;
-      m_uidlsToMark.AppendElement(uidlEntry);
-      rv = NS_OK;
-    } else
-      PR_Free(uidlEntry);
+  uidlEntry->uidl = strdup(aUidl);
+  if (NS_UNLIKELY(!uidlEntry->uidl)) {
+    PR_Free(uidlEntry);
+    return NS_ERROR_OUT_OF_MEMORY;
   }
-  return rv;
+
+  uidlEntry->status = (aMark == POP3_DELETE) ? DELETE_CHAR :
+                      (aMark == POP3_FETCH_BODY) ? FETCH_BODY : KEEP;
+  m_uidlsToMark.AppendElement(uidlEntry);
+  return NS_OK;
 }
 
 NS_IMETHODIMP nsPop3IncomingServer::MarkMessages()
 {
   nsresult rv;
   if (m_runningProtocol)
     rv = m_runningProtocol->MarkMessages(&m_uidlsToMark);
   else