Bug 809064 - Take care error path to avoid the usage of uninitialised local variable 'fieldType'. r=irving,a=Standard8
authorISHIKAWA, Chiaki <ishikawa@yk.rim.or.jp>
Mon, 26 Nov 2012 20:41:40 -0500
changeset 13591 3cc88679f0b7f9001e83cd8db32dc5d447046f5a
parent 13590 733a8a1c3498e362a0423dc7ff5d6e187cd52398
child 13592 c96629272e7f187ee03ce158e0b7f91176eae20a
push id20
push userbugzilla@standard8.plus.com
push dateMon, 31 Dec 2012 16:13:42 +0000
reviewersirving, Standard8
bugs809064
Bug 809064 - Take care error path to avoid the usage of uninitialised local variable 'fieldType'. r=irving,a=Standard8
mailnews/base/src/nsMsgDBView.cpp
--- a/mailnews/base/src/nsMsgDBView.cpp
+++ b/mailnews/base/src/nsMsgDBView.cpp
@@ -532,17 +532,17 @@ nsresult nsMsgDBView::FetchAccount(nsIMs
   // Cache the account manager?
   nsCOMPtr<nsIMsgAccountManager> accountManager(
     do_GetService(NS_MSGACCOUNTMANAGER_CONTRACTID, &rv));
   NS_ENSURE_SUCCESS(rv, rv);
   nsCOMPtr<nsIMsgAccount> account;
   nsCOMPtr<nsIMsgIncomingServer> server;
   if (!accountKey.IsEmpty())
     rv = accountManager->GetAccount(accountKey, getter_AddRefs(account));
-    
+
   if (account)
   {
     account->GetIncomingServer(getter_AddRefs(server));
   }
   else
   {
     nsCOMPtr<nsIMsgFolder> folder;
     aHdr->GetFolder(getter_AddRefs(folder));
@@ -1739,17 +1739,17 @@ void nsMsgDBView::InsertMsgHdrAt(nsMsgVi
     NS_ERROR("Index for message header insertion out of array range!");
     return;
   }
   m_keys.InsertElementAt(index, msgKey);
   m_flags.InsertElementAt(index, flags);
   m_levels.InsertElementAt(index, level);
 }
 
-void nsMsgDBView::SetMsgHdrAt(nsIMsgDBHdr *hdr, nsMsgViewIndex index, 
+void nsMsgDBView::SetMsgHdrAt(nsIMsgDBHdr *hdr, nsMsgViewIndex index,
                               nsMsgKey msgKey, uint32_t flags, uint32_t level)
 {
   m_keys[index] = msgKey;
   m_flags[index] = flags;
   m_levels[index] = level;
 }
 
 nsresult nsMsgDBView::GetFolderForViewIndex(nsMsgViewIndex index, nsIMsgFolder **aFolder)
@@ -1916,17 +1916,17 @@ NS_IMETHODIMP nsMsgDBView::AddColumnHand
     m_customColumnHandlers.ReplaceObjectAt(handler, index);
 
   }
   // Check if the column name matches any of the columns in
   // m_sortColumns, and if so, set m_sortColumns[i].mColHandler
   for (uint32_t i = 0; i < m_sortColumns.Length(); i++)
   {
     MsgViewSortColumnInfo &sortInfo = m_sortColumns[i];
-    if (sortInfo.mSortType == nsMsgViewSortType::byCustom && 
+    if (sortInfo.mSortType == nsMsgViewSortType::byCustom &&
           sortInfo.mCustomColumnName.Equals(column))
       sortInfo.mColHandler = handler;
   }
   return NS_OK;
 }
 
 //remove a custom column handler
 NS_IMETHODIMP nsMsgDBView::RemoveColumnHandler(const nsAString& aColID)
@@ -1940,17 +1940,17 @@ NS_IMETHODIMP nsMsgDBView::RemoveColumnH
   {
     m_customColumnHandlerIDs.RemoveElementAt(index);
     m_customColumnHandlers.RemoveObjectAt(index);
     // Check if the column name matches any of the columns in
     // m_sortColumns, and if so, clear m_sortColumns[i].mColHandler
     for (uint32_t i = 0; i < m_sortColumns.Length(); i++)
     {
       MsgViewSortColumnInfo &sortInfo = m_sortColumns[i];
-      if (sortInfo.mSortType == nsMsgViewSortType::byCustom && 
+      if (sortInfo.mSortType == nsMsgViewSortType::byCustom &&
             sortInfo.mCustomColumnName.Equals(aColID))
         sortInfo.mColHandler = nullptr;
     }
 
     return NS_OK;
   }
 
   return NS_ERROR_FAILURE; //can't remove a column that isn't currently custom handled
@@ -2469,17 +2469,17 @@ NS_IMETHODIMP nsMsgDBView::GetMsgHdrsFor
 }
 
 NS_IMETHODIMP nsMsgDBView::GetURIsForSelection(uint32_t *length, char ***uris)
 {
   nsresult rv = NS_OK;
 
   NS_ENSURE_ARG_POINTER(length);
   *length = 0;
-  
+
   NS_ENSURE_ARG_POINTER(uris);
   *uris = nullptr;
 
   nsMsgViewIndexArray selection;
   GetSelectedIndices(selection);
   uint32_t numIndices = selection.Length();
   if (!numIndices) return NS_OK;
 
@@ -2502,17 +2502,17 @@ NS_IMETHODIMP nsMsgDBView::GetURIsForSel
     nsMsgKey msgKey;
     msgHdr->GetMessageKey(&msgKey);
     msgHdr->GetFolder(getter_AddRefs(folder));
     rv = GenerateURIForMsgKey(msgKey, folder, tmpUri);
     NS_ENSURE_SUCCESS(rv,rv);
     *next = ToNewCString(tmpUri);
     if (!*next)
       return NS_ERROR_OUT_OF_MEMORY;
-    
+
     next++;
   }
 
   *uris = outArray;
   return NS_OK;
 }
 
 NS_IMETHODIMP nsMsgDBView::GetURIForViewIndex(nsMsgViewIndex index, nsACString &result)
@@ -2751,22 +2751,22 @@ NS_IMETHODIMP nsMsgDBView::GetCommandSta
   default:
     NS_ASSERTION(false, "invalid command type");
     rv = NS_ERROR_FAILURE;
   }
   return rv;
 }
 
 // This method needs to be overridden by the various view classes
-// that have different kinds of threads. For example, in a 
+// that have different kinds of threads. For example, in a
 // threaded quick search db view, we'd only want to include children
 // of the thread that fit the view (IMO). And when we have threaded
 // cross folder views, we would include all the children of the
 // cross-folder thread.
-nsresult nsMsgDBView::ListCollapsedChildren(nsMsgViewIndex viewIndex, 
+nsresult nsMsgDBView::ListCollapsedChildren(nsMsgViewIndex viewIndex,
                                             nsIMutableArray *messageArray)
 {
   nsCOMPtr<nsIMsgDBHdr> msgHdr;
   nsCOMPtr<nsIMsgThread> thread;
   GetMsgHdrForViewIndex(viewIndex, getter_AddRefs(msgHdr));
   if (!msgHdr)
   {
     NS_ASSERTION(false, "couldn't find message to expand");
@@ -2833,17 +2833,17 @@ nsresult nsMsgDBView::GetHeadersFromSele
         rv = ListCollapsedChildren(viewIndex, messageArray);
       continue;
     }
     nsCOMPtr<nsIMsgDBHdr> msgHdr;
     rv = GetMsgHdrForViewIndex(viewIndex, getter_AddRefs(msgHdr));
     if (NS_SUCCEEDED(rv) && msgHdr)
     {
       rv = messageArray->AppendElement(msgHdr, false);
-      if (NS_SUCCEEDED(rv) && includeCollapsedMsgs && 
+      if (NS_SUCCEEDED(rv) && includeCollapsedMsgs &&
           viewIndexFlags & nsMsgMessageFlags::Elided &&
           viewIndexFlags & MSG_VIEW_FLAG_HASCHILDREN &&
           m_viewFlags & nsMsgViewFlagsType::kThreadedDisplay)
       {
         rv = ListCollapsedChildren(viewIndex, messageArray);
       }
     }
   }
@@ -3509,17 +3509,17 @@ nsMsgDBView::PerformActionsOnJunkMsgs(bo
     }
     else if (msgsAreJunk)
     {
       if (mDeleteModel == nsMsgImapDeleteModels::IMAPDelete)
       {
         // Unfortunately the DeleteMessages in this case is interpreted by IMAP as
         //  a delete toggle. So what we have to do is to assemble a new delete
         //  array, keeping only those that are not deleted.
-        // 
+        //
         nsCOMPtr<nsIMutableArray> hdrsToDelete = do_CreateInstance("@mozilla.org/array;1", &rv);
         NS_ENSURE_SUCCESS(rv, rv);
         uint32_t cnt;
         rv = mJunkHdrs->GetLength(&cnt);
         for (uint32_t i = 0; i < cnt; i++)
         {
           nsCOMPtr<nsIMsgDBHdr> msgHdr = do_QueryElementAt(mJunkHdrs, i);
           if (msgHdr)
@@ -3720,17 +3720,17 @@ void nsMsgDBView::ReverseThreads()
 
 void nsMsgDBView::ReverseSort()
 {
     uint32_t topIndex = GetSize();
 
     nsCOMArray<nsIMsgFolder> *folders = GetFolders();
 
     // go up half the array swapping values
-    for (uint32_t bottomIndex = 0; bottomIndex < --topIndex; bottomIndex++) 
+    for (uint32_t bottomIndex = 0; bottomIndex < --topIndex; bottomIndex++)
     {
         // swap flags
         uint32_t tempFlags = m_flags[bottomIndex];
         m_flags[bottomIndex] = m_flags[topIndex];
         m_flags[topIndex] = tempFlags;
 
         // swap keys
         nsMsgKey tempKey = m_keys[bottomIndex];
@@ -3825,16 +3825,23 @@ nsMsgDBView::FnSortIdUint32(const void *
 //To compensate for memory alignment required for
 //systems such as HP-UX these values must be 4 bytes
 //aligned.  Don't break this when modify the constants
 const int kMaxSubjectKey = 160;
 const int kMaxLocationKey = 160;  // also used for account
 const int kMaxAuthorKey = 160;
 const int kMaxRecipientKey = 80;
 
+//
+// There are cases when pFieldType is not set:
+// one case returns NS_ERROR_UNEXPECTED;
+// the other case now return NS_ERROR_NULL_POINTER (this is only when
+// colHandler below is null, but is very unlikely).
+// The latter case used to return NS_OK, which was incorrect.
+//
 nsresult nsMsgDBView::GetFieldTypeAndLenForSort(nsMsgViewSortTypeValue sortType, uint16_t *pMaxLen, eFieldType *pFieldType)
 {
     NS_ENSURE_ARG_POINTER(pMaxLen);
     NS_ENSURE_ARG_POINTER(pFieldType);
 
     switch (sortType)
     {
         case nsMsgViewSortType::bySubject:
@@ -3884,20 +3891,30 @@ nsresult nsMsgDBView::GetFieldTypeAndLen
               *pMaxLen = kMaxRecipientKey; //80 - do we need a seperate k?
             }
             else
             {
               *pFieldType = kU32;
               *pMaxLen = 0;
             }
           }
+          else
+          {
+            NS_WARNING("colHandler is null. *pFieldType is not set.");
+            return NS_ERROR_NULL_POINTER;
+          }
           break;
         }
         default:
-            return NS_ERROR_UNEXPECTED;
+        {
+           nsAutoCString message("unexpected switch value: sortType=");
+           message.AppendInt(sortType);
+           NS_WARNING(message.get());
+           return NS_ERROR_UNEXPECTED;
+        }
     }
 
     return NS_OK;
 }
 
 #define MSG_STATUS_MASK (nsMsgMessageFlags::Replied | nsMsgMessageFlags::Forwarded)
 
 nsresult nsMsgDBView::GetStatusSortValue(nsIMsgDBHdr *msgHdr, uint32_t *result)
@@ -4052,17 +4069,17 @@ MsgViewSortColumnInfo::MsgViewSortColumn
   mSortType = other.mSortType;
   mSortOrder = other.mSortOrder;
   mCustomColumnName = other.mCustomColumnName;
   mColHandler = other.mColHandler;
 }
 
 bool MsgViewSortColumnInfo::operator== (const MsgViewSortColumnInfo& other) const
 {
-  return (mSortType == nsMsgViewSortType::byCustom) ? 
+  return (mSortType == nsMsgViewSortType::byCustom) ?
     mCustomColumnName.Equals(other.mCustomColumnName) : mSortType == other.mSortType;
 }
 
 nsresult nsMsgDBView::EncodeColumnSort(nsString &columnSortString)
 {
   for (uint32_t i = 0; i < m_sortColumns.Length(); i++)
   {
     MsgViewSortColumnInfo &sortInfo = m_sortColumns[i];
@@ -4241,67 +4258,73 @@ nsresult nsMsgDBView::SaveSortInfo(nsMsg
     nsCOMPtr <nsIDBFolderInfo> folderInfo;
     nsCOMPtr <nsIMsgDatabase> db;
     nsresult rv = m_viewFolder->GetDBFolderInfoAndDB(getter_AddRefs(folderInfo), getter_AddRefs(db));
     if (NS_SUCCEEDED(rv) && folderInfo)
     {
       // save off sort type and order, view type and flags
       folderInfo->SetSortType(sortType);
       folderInfo->SetSortOrder(sortOrder);
-      
+
       nsString sortColumnsString;
       rv = EncodeColumnSort(sortColumnsString);
       NS_ENSURE_SUCCESS(rv, rv);
       folderInfo->SetProperty("sortColumns", sortColumnsString);
     }
   }
   return NS_OK;
 }
 
 int32_t  nsMsgDBView::SecondarySort(nsMsgKey key1, nsISupports *supports1, nsMsgKey key2, nsISupports *supports2, viewSortInfo *comparisonContext)
 {
-  
+
   // we need to make sure that in the case of the secondary sort field also matching,
-  // we don't recurse 
+  // we don't recurse
   if (comparisonContext->isSecondarySort)
     return key1 > key2;
-  
+
   nsCOMPtr<nsIMsgFolder> folder1, folder2;
   nsCOMPtr <nsIMsgDBHdr> hdr1, hdr2;
   folder1 = do_QueryInterface(supports1);
   folder2 = do_QueryInterface(supports2);
   nsresult rv = folder1->GetMessageHeader(key1, getter_AddRefs(hdr1));
   NS_ENSURE_SUCCESS(rv, 0);
   rv = folder2->GetMessageHeader(key2, getter_AddRefs(hdr2));
   NS_ENSURE_SUCCESS(rv, 0);
   IdKeyPtr EntryInfo1, EntryInfo2;
   EntryInfo1.key = nullptr;
   EntryInfo2.key = nullptr;
 
   uint16_t  maxLen;
   eFieldType fieldType;
   nsMsgViewSortTypeValue sortType = comparisonContext->view->m_secondarySort;
   nsMsgViewSortOrderValue sortOrder = comparisonContext->view->m_secondarySortOrder;
+  // The following may leave fieldType undefined.
+  // In this case, we can return 0 right away since
+  // it is the value returned in the default case of
+  // switch (fieldType) statement below.
   rv = GetFieldTypeAndLenForSort(sortType, &maxLen, &fieldType);
+  NS_ENSURE_SUCCESS(rv, 0);
+
   const void *pValue1 = &EntryInfo1, *pValue2 = &EntryInfo2;
-  
+
   int (* comparisonFun) (const void *pItem1, const void *pItem2, void *privateData) = nullptr;
   int retStatus = 0;
   hdr1->GetMessageKey(&EntryInfo1.id);
   hdr2->GetMessageKey(&EntryInfo2.id);
-  
+
   //check if a custom column handler exists. If it does then grab it and pass it in
   //to either GetCollationKey or GetLongField - we need the custom column handler for
   // the previous sort, if any.
   nsIMsgCustomColumnHandler* colHandler = nullptr; // GetCurColumnHandlerFromDBInfo();
   if (sortType == nsMsgViewSortType::byCustom &&
       comparisonContext->view->m_sortColumns.Length() > 1)
     colHandler = comparisonContext->view->m_sortColumns[1].mColHandler;
 
-  
+
   switch (fieldType)
   {
     case kCollationKey:
       rv = GetCollationKey(hdr1, sortType, &EntryInfo1.key, &EntryInfo1.dword, colHandler);
       NS_ASSERTION(NS_SUCCEEDED(rv),"failed to create collation key");
       comparisonFun = FnSortIdKeyPtr;
       break;
     case kU32:
@@ -4322,21 +4345,21 @@ int32_t  nsMsgDBView::SecondarySort(nsMs
     PR_FREEIF(EntryInfo2.key);
     rv = GetCollationKey(hdr2, sortType, &EntryInfo2.key, &EntryInfo2.dword, colHandler);
     NS_ASSERTION(NS_SUCCEEDED(rv),"failed to create collation key");
   }
   else if (fieldType == kU32)
   {
     if (sortType == nsMsgViewSortType::byId)
       EntryInfo2.dword = EntryInfo2.id;
-    else 
+    else
       GetLongField(hdr2, sortType, &EntryInfo2.dword, colHandler);
   }
   retStatus = (*comparisonFun)(&pValue1, &pValue2, comparisonContext);
-  
+
   comparisonContext->isSecondarySort = false;
   comparisonContext->ascendingSort = saveAscendingSort;
 
   return retStatus;
 }
 
 
 NS_IMETHODIMP nsMsgDBView::Sort(nsMsgViewSortTypeValue sortType, nsMsgViewSortOrderValue sortOrder)
@@ -4346,17 +4369,17 @@ NS_IMETHODIMP nsMsgDBView::Sort(nsMsgVie
   // And the custom column we're sorting on might have changed, so to be
   // on the safe side, resort.
   if (m_sortType == sortType && m_sortValid && sortType != nsMsgViewSortType::byCustom
     && m_sortColumns.Length() < 2)
   {
     // same as it ever was.  do nothing
     if (m_sortOrder == sortOrder)
       return NS_OK;
-    
+
     // for secondary sort, remember the sort order on a per column basis.
     if (m_sortColumns.Length())
       m_sortColumns[0].mSortOrder = sortOrder;
     SaveSortInfo(sortType, sortOrder);
     if (m_viewFlags & nsMsgViewFlagsType::kThreadedDisplay)
       ReverseThreads();
     else
       ReverseSort();
@@ -4377,27 +4400,29 @@ NS_IMETHODIMP nsMsgDBView::Sort(nsMsgVie
     if (sortType == nsMsgViewSortType::byCustom)
     {
       GetCurCustomColumn(sortColumnInfo.mCustomColumnName);
       sortColumnInfo.mColHandler = GetCurColumnHandlerFromDBInfo();
     }
 
     PushSort(sortColumnInfo);
   }
-  
+
   if (m_sortColumns.Length() > 1)
   {
     m_secondarySort = m_sortColumns[1].mSortType;
     m_secondarySortOrder = m_sortColumns[1].mSortOrder;
   }
   SaveSortInfo(sortType, sortOrder);
   // figure out how much memory we'll need, and the malloc it
   uint16_t maxLen;
   eFieldType fieldType;
 
+  // If we did not obtain proper fieldType, it needs to be checked
+  // because the subsequent code does not handle it very well.
   rv = GetFieldTypeAndLenForSort(sortType, &maxLen, &fieldType);
   NS_ENSURE_SUCCESS(rv,rv);
 
   nsVoidArray ptrs;
   uint32_t arraySize = GetSize();
 
   if (!arraySize)
     return NS_OK;
@@ -4452,16 +4477,17 @@ NS_IMETHODIMP nsMsgDBView::Sort(nsMsgVie
     }
 
     //check if a custom column handler exists. If it does then grab it and pass it in
     //to either GetCollationKey or GetLongField
     nsIMsgCustomColumnHandler* colHandler = GetCurColumnHandlerFromDBInfo();
 
     // could be a problem here if the ones that appear here are different than the ones already in the array
     uint32_t actualFieldLen = 0;
+
     if (fieldType == kCollationKey)
     {
       rv = GetCollationKey(msgHdr, sortType, &keyValue, &actualFieldLen, colHandler);
       NS_ENSURE_SUCCESS(rv,rv);
 
       longValue = actualFieldLen;
     }
     else
@@ -4636,17 +4662,17 @@ nsMsgViewIndex nsMsgDBView::GetThreadInd
 
   // scan up looking for level 0 message.
   while (m_levels[msgIndex] && msgIndex)
     --msgIndex;
   return msgIndex;
 }
 
 nsMsgViewIndex
-nsMsgDBView::ThreadIndexOfMsgHdr(nsIMsgDBHdr *msgHdr, 
+nsMsgDBView::ThreadIndexOfMsgHdr(nsIMsgDBHdr *msgHdr,
                                  nsMsgViewIndex msgIndex,
                                  int32_t *pThreadCount,
                                  uint32_t *pFlags)
 {
   nsCOMPtr<nsIMsgThread> threadHdr;
   nsresult rv = GetThreadContainingMsgHdr(msgHdr, getter_AddRefs(threadHdr));
   NS_ENSURE_SUCCESS(rv, nsMsgViewIndex_None);
 
@@ -4862,17 +4888,17 @@ nsresult nsMsgDBView::ToggleExpansion(ns
   // if not a thread, or doesn't have children, no expand/collapse
   // If we add sub-thread expand collapse, this will need to be relaxed
   if (!(flags & MSG_VIEW_FLAG_ISTHREAD) || !(flags & MSG_VIEW_FLAG_HASCHILDREN))
     return NS_MSG_MESSAGE_NOT_FOUND;
   if (flags & nsMsgMessageFlags::Elided)
     rv = ExpandByIndex(threadIndex, numChanged);
   else
     rv = CollapseByIndex(threadIndex, numChanged);
-    
+
   // if we collaps/uncollapse a thread, this changes the selected URIs
   SelectionChanged();
   return rv;
 }
 
 nsresult nsMsgDBView::ExpandAndSelectThread()
 {
     nsresult rv;
@@ -5069,33 +5095,39 @@ nsresult nsMsgDBView::OnNewHeader(nsIMsg
 NS_IMETHODIMP nsMsgDBView::GetThreadContainingIndex(nsMsgViewIndex index, nsIMsgThread **resultThread)
 {
   nsCOMPtr <nsIMsgDBHdr> msgHdr;
   nsresult rv = GetMsgHdrForViewIndex(index, getter_AddRefs(msgHdr));
   NS_ENSURE_SUCCESS(rv, rv);
   return GetThreadContainingMsgHdr(msgHdr, resultThread);
 }
 
-nsMsgViewIndex 
+nsMsgViewIndex
 nsMsgDBView::GetIndexForThread(nsIMsgDBHdr *msgHdr)
 {
   // Take advantage of the fact that we're already sorted
   // and find the insert index via a binary search, though expanded threads
-  // make that tricky.  
+  // make that tricky.
 
   nsMsgViewIndex highIndex = m_keys.Length();
   nsMsgViewIndex lowIndex = 0;
   IdKeyPtr EntryInfo1, EntryInfo2;
   EntryInfo1.key = nullptr;
   EntryInfo2.key = nullptr;
 
   nsresult rv;
   uint16_t  maxLen;
   eFieldType fieldType;
+  // The following may leave fieldType undefined.
+  // In this case, we can return highIndex right away since
+  // it is the value returned in the default case of
+  // switch (fieldType) statement below.
   rv = GetFieldTypeAndLenForSort(m_sortType, &maxLen, &fieldType);
+  NS_ENSURE_SUCCESS(rv, highIndex);
+
   const void *pValue1 = &EntryInfo1, *pValue2 = &EntryInfo2;
 
   int retStatus = 0;
   msgHdr->GetMessageKey(&EntryInfo1.id);
   msgHdr->GetFolder(&EntryInfo1.folder);
   EntryInfo1.folder->Release();
   //check if a custom column handler exists. If it does then grab it and pass it in
   //to either GetCollationKey or GetLongField
@@ -5133,17 +5165,17 @@ nsMsgDBView::GetIndexForThread(nsIMsgDBH
     if (tryIndex < lowIndex)
     {
       NS_ERROR("try index shouldn't be less than low index");
       break;
     }
     EntryInfo2.id = m_keys[tryIndex];
     GetFolderForViewIndex(tryIndex, &EntryInfo2.folder);
     EntryInfo2.folder->Release();
-    
+
     nsCOMPtr <nsIMsgDBHdr> tryHdr;
     nsCOMPtr <nsIMsgDatabase> db;
     // ### this should get the db from the folder...
     GetDBForViewIndex(tryIndex, getter_AddRefs(db));
     if (db)
       rv = db->GetMsgHdrForKey(EntryInfo2.id, getter_AddRefs(tryHdr));
     if (!tryHdr)
       break;
@@ -5200,17 +5232,23 @@ nsMsgViewIndex nsMsgDBView::GetInsertInd
   nsMsgViewIndex lowIndex = 0;
   IdKeyPtr EntryInfo1, EntryInfo2;
   EntryInfo1.key = nullptr;
   EntryInfo2.key = nullptr;
 
   nsresult rv;
   uint16_t  maxLen;
   eFieldType fieldType;
+  // The following may leave fieldType undefined.
+  // In this case, we can return highIndex right away since
+  // it is the value returned in the default case of
+  // switch (fieldType) statement below.
   rv = GetFieldTypeAndLenForSort(sortType, &maxLen, &fieldType);
+  NS_ENSURE_SUCCESS(rv, highIndex);
+
   const void *pValue1 = &EntryInfo1, *pValue2 = &EntryInfo2;
 
   int (* comparisonFun) (const void *pItem1, const void *pItem2, void *privateData) = nullptr;
   int retStatus = 0;
   msgHdr->GetMessageKey(&EntryInfo1.id);
   msgHdr->GetFolder(&EntryInfo1.folder);
   EntryInfo1.folder->Release();
   //check if a custom column handler exists. If it does then grab it and pass it in
@@ -5241,17 +5279,17 @@ nsMsgViewIndex nsMsgDBView::GetInsertInd
     default:
       return highIndex;
   }
   while (highIndex > lowIndex)
   {
     nsMsgViewIndex tryIndex = (lowIndex + highIndex - 1) / 2;
     EntryInfo2.id = keys[tryIndex];
     EntryInfo2.folder = folders ? folders->ObjectAt(tryIndex) : m_folder.get();
-    
+
     nsCOMPtr <nsIMsgDBHdr> tryHdr;
     EntryInfo2.folder->GetMessageHeader(EntryInfo2.id, getter_AddRefs(tryHdr));
     if (!tryHdr)
       break;
     if (fieldType == kCollationKey)
     {
       PR_FREEIF(EntryInfo2.key);
       rv = GetCollationKey(tryHdr, sortType, &EntryInfo2.key, &EntryInfo2.dword, colHandler);
@@ -5463,18 +5501,18 @@ nsresult nsMsgDBView::ListIdsInThreadOrd
     }
   }
   return rv; // we don't want to return the rv from the enumerator when it reaches the end, do we?
 }
 
 bool nsMsgDBView::InsertEmptyRows(nsMsgViewIndex viewIndex, int32_t numRows)
 {
   return m_keys.InsertElementsAt(viewIndex, numRows, 0) &&
-         m_flags.InsertElementsAt(viewIndex, numRows, 0) && 
-         m_levels.InsertElementsAt(viewIndex, numRows, 1); 
+         m_flags.InsertElementsAt(viewIndex, numRows, 0) &&
+         m_levels.InsertElementsAt(viewIndex, numRows, 1);
 }
 
 void nsMsgDBView::RemoveRows(nsMsgViewIndex viewIndex, int32_t numRows)
 {
   m_keys.RemoveElementsAt(viewIndex, numRows);
   m_flags.RemoveElementsAt(viewIndex, numRows);
   m_levels.RemoveElementsAt(viewIndex, numRows);
 }
@@ -5604,17 +5642,17 @@ int32_t nsMsgDBView::FindLevelInThread(n
       // key+parentKey will work after first time through loop
       curMsgHdr->GetMessageKey(&msgKey);
     }
   }
   return 1;
 }
 
 // ### Can this be combined with GetIndexForThread??
-nsMsgViewIndex 
+nsMsgViewIndex
 nsMsgDBView::GetThreadRootIndex(nsIMsgDBHdr *msgHdr)
 {
   if (!msgHdr)
   {
     NS_WARNING("null msgHdr parameter");
     return nsMsgViewIndex_None;
   }
 
@@ -5625,17 +5663,23 @@ nsMsgDBView::GetThreadRootIndex(nsIMsgDB
   nsMsgViewIndex lowIndex = 0;
   IdKeyPtr EntryInfo1, EntryInfo2;
   EntryInfo1.key = nullptr;
   EntryInfo2.key = nullptr;
 
   nsresult rv;
   uint16_t maxLen;
   eFieldType fieldType;
+  // The following may leave fieldType undefined.
+  // In this case, we can return highIndex right away since
+  // it is the value returned in the default case of
+  // switch (fieldType) statement below.
   rv = GetFieldTypeAndLenForSort(m_sortType, &maxLen, &fieldType);
+  NS_ENSURE_SUCCESS(rv, highIndex);
+
   const void *pValue1 = &EntryInfo1, *pValue2 = &EntryInfo2;
 
   int retStatus = 0;
   msgHdr->GetMessageKey(&EntryInfo1.id);
   msgHdr->GetFolder(&EntryInfo1.folder);
   EntryInfo1.folder->Release();
   //check if a custom column handler exists. If it does then grab it and pass it in
   //to either GetCollationKey or GetLongField
@@ -5673,17 +5717,17 @@ nsMsgDBView::GetThreadRootIndex(nsIMsgDB
     if (tryIndex < lowIndex)
     {
       NS_ERROR("try index shouldn't be less than low index");
       break;
     }
     EntryInfo2.id = m_keys[tryIndex];
     GetFolderForViewIndex(tryIndex, &EntryInfo2.folder);
     EntryInfo2.folder->Release();
-    
+
     nsCOMPtr<nsIMsgDBHdr> tryHdr;
     nsCOMPtr<nsIMsgDatabase> db;
     // ### this should get the db from the folder...
     GetDBForViewIndex(tryIndex, getter_AddRefs(db));
     if (db)
       rv = db->GetMsgHdrForKey(EntryInfo2.id, getter_AddRefs(tryHdr));
     if (!tryHdr)
       break;
@@ -5752,17 +5796,19 @@ nsMsgDBView::GetThreadRootIndex(nsIMsgDB
 
 void nsMsgDBView::InitEntryInfoForIndex(nsMsgViewIndex i, IdKeyPtr &EntryInfo)
 {
   EntryInfo.key = nullptr;
 
   nsresult rv;
   uint16_t maxLen;
   eFieldType fieldType;
+  // The following may leave fieldType undefined.
   rv = GetFieldTypeAndLenForSort(m_sortType, &maxLen, &fieldType);
+  NS_ASSERTION(NS_SUCCEEDED(rv),"failed to obtain fieldType");
 
   nsCOMPtr<nsIMsgDBHdr> msgHdr;
   GetMsgHdrForViewIndex(i, getter_AddRefs(msgHdr));
 
   msgHdr->GetMessageKey(&EntryInfo.id);
   msgHdr->GetFolder(&EntryInfo.folder);
   EntryInfo.folder->Release();
   //check if a custom column handler exists. If it does then grab it and pass it in
@@ -5791,17 +5837,23 @@ void nsMsgDBView::InitEntryInfoForIndex(
 
 void nsMsgDBView::ValidateSort()
 {
   IdKeyPtr EntryInfo1, EntryInfo2;
   nsCOMPtr<nsIMsgDBHdr> hdr1, hdr2;
 
   uint16_t  maxLen;
   eFieldType fieldType;
-  GetFieldTypeAndLenForSort(m_sortType, &maxLen, &fieldType);
+  // It is not entirely clear what we should do since,
+  // if fieldType is not available, there is no way to know
+  // how to compare the field to check for sorting.
+  // So we bomb out here. It is OK since this is debug code
+  // inside  #ifdef DEBUG_David_Bienvenu
+  rv = GetFieldTypeAndLenForSort(m_sortType, &maxLen, &fieldType);
+  NS_ASSERTION(NS_SUCCEEDED(rv),"failed to obtain fieldType");
 
   viewSortInfo comparisonContext;
   comparisonContext.view = this;
   comparisonContext.isSecondarySort = false;
   comparisonContext.ascendingSort = (m_sortOrder == nsMsgViewSortOrder::ascending);
   nsCOMPtr<nsIMsgDatabase> db;
   GetDBForViewIndex(0, getter_AddRefs(db));
   // this is only for comparing collation keys - it could be any db.
@@ -5954,17 +6006,17 @@ NS_IMETHODIMP nsMsgDBView::OnHdrAdded(ns
                           nsIDBChangeListener *aInstigator)
 {
   return OnNewHeader(aHdrChanged, aParentKey, false);
   // probably also want to pass that parent key in, since we went to the trouble
   // of figuring out what it is.
 }
 
 NS_IMETHODIMP
-nsMsgDBView::OnHdrPropertyChanged(nsIMsgDBHdr *aHdrToChange, bool aPreChange, uint32_t *aStatus, 
+nsMsgDBView::OnHdrPropertyChanged(nsIMsgDBHdr *aHdrToChange, bool aPreChange, uint32_t *aStatus,
                                  nsIDBChangeListener * aInstigator)
 {
   if (aPreChange)
     return NS_OK;
 
   if (aHdrToChange)
   {
     nsMsgViewIndex index = FindHdr(aHdrToChange);
@@ -6907,17 +6959,17 @@ nsresult nsMsgDBView::SetSubthreadKilled
   if (!IsValidIndex(msgIndex))
     return NS_MSG_INVALID_DBVIEW_INDEX;
 
   NoteChange(msgIndex, 1, nsMsgViewNotificationCode::changed);
   nsresult rv;
 
   rv = m_db->MarkHeaderKilled(header, ignored, this);
   NS_ENSURE_SUCCESS(rv, rv);
- 
+
   if (ignored)
   {
     nsCOMPtr <nsIMsgThread> thread;
     nsresult rv;
     rv = GetThreadContainingMsgHdr(header, getter_AddRefs(thread));
     if (NS_FAILED(rv))
       return NS_OK; // So we didn't mark threads read
 
@@ -6935,20 +6987,20 @@ nsresult nsMsgDBView::SetSubthreadKilled
          break;
     }
 
     // Process all messages, starting with this message.
     for (; current < children; current++)
     {
        nsCOMPtr <nsIMsgDBHdr> nextHdr;
        bool isKilled;
-       
+
        thread->GetChildHdrAt(current, getter_AddRefs(nextHdr));
        nextHdr->GetIsKilled(&isKilled);
-       
+
        // Ideally, the messages should stop processing here.
        // However, the children are ordered not by thread...
        if (isKilled)
          nextHdr->MarkRead(true);
     }
   }
   return NS_OK;
 }
@@ -7024,17 +7076,17 @@ nsMsgDBView::GetNumSelected(uint32_t *aN
 
 NS_IMETHODIMP nsMsgDBView::GetNumMsgsInView(int32_t *aNumMsgs)
 {
   NS_ENSURE_ARG_POINTER(aNumMsgs);
   return (m_folder) ? m_folder->GetTotalMessages(false, aNumMsgs) :
                     NS_ERROR_FAILURE;
 }
 /**
- * @note For the IMAP delete model, this applies to both deleting and 
+ * @note For the IMAP delete model, this applies to both deleting and
  *       undeleting a message.
  */
 NS_IMETHODIMP
 nsMsgDBView::GetMsgToSelectAfterDelete(nsMsgViewIndex *msgToSelectAfterDelete)
 {
   NS_ENSURE_ARG_POINTER(msgToSelectAfterDelete);
   *msgToSelectAfterDelete = nsMsgViewIndex_None;
 
@@ -7052,25 +7104,25 @@ nsMsgDBView::GetMsgToSelectAfterDelete(n
     int32_t selectionCount;
     int32_t startRange;
     int32_t endRange;
     nsresult rv = mTreeSelection->GetRangeCount(&selectionCount);
     for (int32_t i = 0; i < selectionCount; i++)
     {
       rv = mTreeSelection->GetRangeAt(i, &startRange, &endRange);
 
-      // save off the first range in case we need it later 
+      // save off the first range in case we need it later
       if (i == 0) {
         startFirstRange = startRange;
         endFirstRange = endRange;
       } else {
         // If the tree selection is goofy (eg adjacent or overlapping ranges),
         // complain about it, but don't try and cope.  Just live with the fact
         // that one of the deleted messages is going to end up selected.
-        NS_WARN_IF_FALSE(endFirstRange != startRange, 
+        NS_WARN_IF_FALSE(endFirstRange != startRange,
                          "goofy tree selection state: two ranges are adjacent!");
       }
       *msgToSelectAfterDelete = NS_MIN(*msgToSelectAfterDelete,
                                        (nsMsgViewIndex)startRange);
     }
 
     // Multiple selection either using Ctrl, Shift, or one of the affordances
     // to select an entire thread.
@@ -7244,17 +7296,17 @@ bool nsMsgDBView::OfflineMsgSelected(nsM
       return true;
   }
   return false;
 }
 
 bool nsMsgDBView::NonDummyMsgSelected(nsMsgViewIndex * indices, int32_t numIndices)
 {
   bool includeCollapsedMsgs = OperateOnMsgsInCollapsedThreads();
-  
+
   for (nsMsgViewIndex index = 0; index < (nsMsgViewIndex) numIndices; index++)
   {
     uint32_t flags = m_flags[indices[index]];
     // We now treat having a collapsed dummy message selected as if
     // the whole group was selected so we can apply commands to the group.
     if (!(flags & MSG_VIEW_FLAG_DUMMY) ||
         (flags & nsMsgMessageFlags::Elided && includeCollapsedMsgs))
       return true;