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 9918 95b1e9e741571caa128dbec50a04878844a62004
parent 9917 ccef1e7af3cb9bfe2be09f1a8e14ec46858cc063
child 9919 340fc94f25b69316d1dd33ff9e165b6c78d9b8b3
push id44
push userbugzilla@standard8.plus.com
push dateThu, 03 Jan 2013 11:28:28 +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
@@ -566,17 +566,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));
@@ -1793,17 +1793,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, PRUint32 flags, PRUint32 level)
 {
   m_keys[index] = msgKey;
   m_flags[index] = flags;
   m_levels[index] = level;
 }
 
 nsresult nsMsgDBView::GetFolderForViewIndex(nsMsgViewIndex index, nsIMsgFolder **aFolder)
@@ -1970,17 +1970,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 (PRUint32 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)
@@ -1994,17 +1994,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 (PRUint32 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 = nsnull;
     }
 
     return NS_OK;
   }
 
   return NS_ERROR_FAILURE; //can't remove a column that isn't currently custom handled
@@ -2523,17 +2523,17 @@ NS_IMETHODIMP nsMsgDBView::GetMsgHdrsFor
 }
 
 NS_IMETHODIMP nsMsgDBView::GetURIsForSelection(PRUint32 *length, char ***uris)
 {
   nsresult rv = NS_OK;
 
   NS_ENSURE_ARG_POINTER(length);
   *length = 0;
-  
+
   NS_ENSURE_ARG_POINTER(uris);
   *uris = nsnull;
 
   nsMsgViewIndexArray selection;
   GetSelectedIndices(selection);
   PRUint32 numIndices = selection.Length();
   if (!numIndices) return NS_OK;
 
@@ -2556,17 +2556,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)
@@ -2805,22 +2805,22 @@ NS_IMETHODIMP nsMsgDBView::GetCommandSta
   default:
     NS_ASSERTION(PR_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(PR_FALSE, "couldn't find message to expand");
@@ -2887,17 +2887,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, PR_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);
       }
     }
   }
@@ -3753,17 +3753,17 @@ void nsMsgDBView::ReverseThreads()
 
 void nsMsgDBView::ReverseSort()
 {
     PRUint32 topIndex = GetSize();
 
     nsCOMArray<nsIMsgFolder> *folders = GetFolders();
 
     // go up half the array swapping values
-    for (PRUint32 bottomIndex = 0; bottomIndex < --topIndex; bottomIndex++) 
+    for (PRUint32 bottomIndex = 0; bottomIndex < --topIndex; bottomIndex++)
     {
         // swap flags
         PRUint32 tempFlags = m_flags[bottomIndex];
         m_flags[bottomIndex] = m_flags[topIndex];
         m_flags[topIndex] = tempFlags;
 
         // swap keys
         nsMsgKey tempKey = m_keys[bottomIndex];
@@ -3858,16 +3858,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, PRUint16 *pMaxLen, eFieldType *pFieldType)
 {
     NS_ENSURE_ARG_POINTER(pMaxLen);
     NS_ENSURE_ARG_POINTER(pFieldType);
 
     switch (sortType)
     {
         case nsMsgViewSortType::bySubject:
@@ -3917,20 +3924,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, PRUint32 *result)
@@ -4085,17 +4102,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 (PRUint32 i = 0; i < m_sortColumns.Length(); i++)
   {
     MsgViewSortColumnInfo &sortInfo = m_sortColumns[i];
@@ -4274,67 +4291,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;
 }
 
 PRInt32  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 = nsnull;
   EntryInfo2.key = nsnull;
 
   PRUint16  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) = nsnull;
   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 = nsnull; // 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:
@@ -4355,21 +4378,21 @@ PRInt32  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 = PR_FALSE;
   comparisonContext->ascendingSort = saveAscendingSort;
 
   return retStatus;
 }
 
 
 NS_IMETHODIMP nsMsgDBView::Sort(nsMsgViewSortTypeValue sortType, nsMsgViewSortOrderValue sortOrder)
@@ -4379,17 +4402,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();
@@ -4410,27 +4433,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
   PRUint16 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;
   PRUint32 arraySize = GetSize();
 
   if (!arraySize)
     return NS_OK;
@@ -4485,16 +4510,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
     PRUint32 actualFieldLen = 0;
+
     if (fieldType == kCollationKey)
     {
       rv = GetCollationKey(msgHdr, sortType, &keyValue, &actualFieldLen, colHandler);
       NS_ENSURE_SUCCESS(rv,rv);
 
       longValue = actualFieldLen;
     }
     else
@@ -4669,17 +4695,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,
                                  PRInt32 *pThreadCount,
                                  PRUint32 *pFlags)
 {
   nsCOMPtr<nsIMsgThread> threadHdr;
   nsresult rv = GetThreadContainingMsgHdr(msgHdr, getter_AddRefs(threadHdr));
   NS_ENSURE_SUCCESS(rv, nsMsgViewIndex_None);
 
@@ -4894,17 +4920,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;
@@ -5094,33 +5120,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 = nsnull;
   EntryInfo2.key = nsnull;
 
   nsresult rv;
   PRUint16  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
@@ -5158,17 +5190,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;
@@ -5225,17 +5257,23 @@ nsMsgViewIndex nsMsgDBView::GetInsertInd
   nsMsgViewIndex lowIndex = 0;
   IdKeyPtr EntryInfo1, EntryInfo2;
   EntryInfo1.key = nsnull;
   EntryInfo2.key = nsnull;
 
   nsresult rv;
   PRUint16  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) = nsnull;
   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
@@ -5266,17 +5304,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);
@@ -5486,18 +5524,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, PRInt32 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, PRInt32 numRows)
 {
   m_keys.RemoveElementsAt(viewIndex, numRows);
   m_flags.RemoveElementsAt(viewIndex, numRows);
   m_levels.RemoveElementsAt(viewIndex, numRows);
 }
@@ -5627,17 +5665,17 @@ PRInt32 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;
   }
 
@@ -5648,17 +5686,23 @@ nsMsgDBView::GetThreadRootIndex(nsIMsgDB
   nsMsgViewIndex lowIndex = 0;
   IdKeyPtr EntryInfo1, EntryInfo2;
   EntryInfo1.key = nsnull;
   EntryInfo2.key = nsnull;
 
   nsresult rv;
   PRUint16 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
@@ -5696,17 +5740,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;
@@ -5775,17 +5819,19 @@ nsMsgDBView::GetThreadRootIndex(nsIMsgDB
 
 void nsMsgDBView::InitEntryInfoForIndex(nsMsgViewIndex i, IdKeyPtr &EntryInfo)
 {
   EntryInfo.key = nsnull;
 
   nsresult rv;
   PRUint16 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
@@ -5814,17 +5860,23 @@ void nsMsgDBView::InitEntryInfoForIndex(
 
 void nsMsgDBView::ValidateSort()
 {
   IdKeyPtr EntryInfo1, EntryInfo2;
   nsCOMPtr<nsIMsgDBHdr> hdr1, hdr2;
 
   PRUint16  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 = PR_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.
@@ -5977,17 +6029,17 @@ NS_IMETHODIMP nsMsgDBView::OnHdrAdded(ns
                           nsIDBChangeListener *aInstigator)
 {
   return OnNewHeader(aHdrChanged, aParentKey, PR_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, PRUint32 *aStatus, 
+nsMsgDBView::OnHdrPropertyChanged(nsIMsgDBHdr *aHdrToChange, bool aPreChange, PRUint32 *aStatus,
                                  nsIDBChangeListener * aInstigator)
 {
   if (aPreChange)
     return NS_OK;
 
   if (aHdrToChange)
   {
     nsMsgViewIndex index = FindHdr(aHdrToChange);
@@ -6930,17 +6982,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
 
@@ -6958,20 +7010,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(PR_TRUE);
     }
   }
   return NS_OK;
 }
@@ -7047,17 +7099,17 @@ nsMsgDBView::GetNumSelected(PRUint32 *aN
 
 NS_IMETHODIMP nsMsgDBView::GetNumMsgsInView(PRInt32 *aNumMsgs)
 {
   NS_ENSURE_ARG_POINTER(aNumMsgs);
   return (m_folder) ? m_folder->GetTotalMessages(PR_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;
 
@@ -7074,25 +7126,25 @@ nsMsgDBView::GetMsgToSelectAfterDelete(n
     PRInt32 selectionCount;
     PRInt32 startRange;
     PRInt32 endRange;
     nsresult rv = mTreeSelection->GetRangeCount(&selectionCount);
     for (PRInt32 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.
@@ -7266,17 +7318,17 @@ bool nsMsgDBView::OfflineMsgSelected(nsM
       return PR_TRUE;
   }
   return PR_FALSE;
 }
 
 bool nsMsgDBView::NonDummyMsgSelected(nsMsgViewIndex * indices, PRInt32 numIndices)
 {
   bool includeCollapsedMsgs = OperateOnMsgsInCollapsedThreads();
-  
+
   for (nsMsgViewIndex index = 0; index < (nsMsgViewIndex) numIndices; index++)
   {
     PRUint32 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 PR_TRUE;