1285101 - (coverity) uninitialized scalar variable: mailnews/imap/src/nsImapOfflineSync.cpp: |flagOperation| may not be initialized on the first use.

Status: NEW

(MailNews Core :: Networking: IMAP, defect)

Description

[ISHIKAWA, Chiaki]

Coverity found this:
|flagOperation| may not be initialized on the first use.
(Actually |newFlags| may be also.
The quote below shows [select issue], and in Coverity UI, we can switch the messages for |flagOperation| and |newFlags|.

234void nsImapOfflineSync::ProcessFlagOperation(nsIMsgOfflineImapOperation *op)
235{
236  nsCOMPtr <nsIMsgOfflineImapOperation> currentOp = op;
237  nsTArray<nsMsgKey> matchingFlagKeys;
238  uint32_t currentKeyIndex = m_KeyIndex;
239
240  imapMessageFlagsType matchingFlags;
241  currentOp->GetNewFlags(&matchingFlags);
   1. var_decl: Declaring variable flagOperation without initializer.
242  imapMessageFlagsType flagOperation;
243  imapMessageFlagsType newFlags;
244  bool flagsMatch = true;
245  do
246  { // loop for all messsages with the same flags
   2. Condition flagsMatch, taking true branch
247    if (flagsMatch)
248    {
249      nsMsgKey curKey;
250      currentOp->GetMessageKey(&curKey);
251      matchingFlagKeys.AppendElement(curKey);
252      currentOp->SetPlayingBack(true);
253      m_currentOpsToClear.AppendObject(currentOp);
254    }
255    currentOp = nullptr;
   3. Condition ++currentKeyIndex < this->m_CurrentKeys.Length(), taking true branch
256    if (++currentKeyIndex < m_CurrentKeys.Length())
257      m_currentDB->GetOfflineOpForKey(m_CurrentKeys[currentKeyIndex], false,
258        getter_AddRefs(currentOp));
   4. Condition currentOp.operator bool(), taking false branch
259    if (currentOp)
260    {
261      currentOp->GetFlagOperation(&flagOperation);
262      currentOp->GetNewFlags(&newFlags);
263    }
   CID 451007: Uninitialized scalar variable (UNINIT) [select issue]
   CID 450452 (#1 of 1): Uninitialized scalar variable (UNINIT)5. uninit_use: Using uninitialized value flagOperation.
264    flagsMatch = (flagOperation & nsIMsgOfflineImapOperation::kFlagsChanged)
265                  && (newFlags == matchingFlags);
266  } while (currentOp);
267
268  if (!matchingFlagKeys.IsEmpty())
269  {
270    nsAutoCString uids;
271    nsImapMailFolder::AllocateUidStringFromKeys(matchingFlagKeys.Elements(), matchingFlagKeys.Length(), uids);
272    uint32_t curFolderFlags;
273    m_currentFolder->GetFlags(&curFolderFlags);
274
275    if (uids.get() && (curFolderFlags & nsMsgFolderFlags::ImapBox)) 
276    {
277      nsresult rv = NS_OK;
278      nsCOMPtr <nsIMsgImapMailFolder> imapFolder = do_QueryInterface(m_currentFolder);
279      nsCOMPtr <nsIURI> uriToSetFlags;
280      if (imapFolder)
281      {
282        rv = imapFolder->SetImapFlags(uids.get(), matchingFlags, getter_AddRefs(uriToSetFlags));
283        if (NS_SUCCEEDED(rv) && uriToSetFlags)
284        {
285          nsCOMPtr <nsIMsgMailNewsUrl> mailnewsUrl = do_QueryInterface(uriToSetFlags);
286          if (mailnewsUrl)
287            mailnewsUrl->RegisterListener(this);
288        }
289      }
290    }
291  }
292  else
293    ProcessNextOperation();
294}
295

Observation:

The code and the underlying assumption about the state of the data is not documented well, and very opaque.
|flagOperation|, |newFlags| may be used uninitailized (?). If there is an intrinsic data dependency that won"t cause this, it should be either commented for reference, or better still, assert()ed to make sure this holds!

Maybe we need to reset flagOperation to 0 on each iteration ?

Comment 1

[ISHIKAWA, Chiaki]

|newFlags| is CD 451007 entry.