1280664 - (coverity) uninitialized scalar variable: mailnews/imap/src/nsImapProtocol.cpp: |rv| is not set always.

Status: NEW

(MailNews Core :: Networking: IMAP, defect)

Description

[ISHIKAWA, Chiaki]

Coverity found this:

Return value |rv| is not initialized always.

2107// LoadImapUrl takes a url, initializes all of our url specific data by calling SetupUrl.
2108// If we don't have a connection yet, we open the connection. Finally, we signal the
2109// url to run monitor to let the imap main thread loop process the current url (it is waiting
2110// on this monitor). There is a contract that the imap thread has already been started b4 we
2111// attempt to load a url....
2112NS_IMETHODIMP nsImapProtocol::LoadImapUrl(nsIURI * aURL, nsISupports * aConsumer)
2113{
    1. var_decl: Declaring variable rv without initializer.
2114  nsresult rv;
    2. Condition aURL, taking false branch
2115  if (aURL)
2116  {
2117#ifdef DEBUG_bienvenu
2118    nsAutoCString urlSpec;
2119    aURL->GetSpec(urlSpec);
2120    printf("loading url %s\n", urlSpec.get());
2121#endif
2122    if (TryToRunUrlLocally(aURL, aConsumer))
2123      return NS_OK;
2124    m_urlInProgress = true;
2125    m_imapMailFolderSink = nullptr;
2126    rv = SetupWithUrl(aURL, aConsumer);
2127    NS_ASSERTION(NS_SUCCEEDED(rv), "error setting up imap url");
2128    if (NS_FAILED(rv))
2129      return rv;
2130
2131    SetupSinkProxy(); // generate proxies for all of the event sinks in the url
2132    m_lastActiveTime = PR_Now();
2133    if (m_transport && m_runningUrl)
2134    {
2135      nsImapAction imapAction;
2136      m_runningUrl->GetImapAction(&imapAction);
2137      // if we're shutting down, and not running the kinds of urls we run at
2138      // shutdown, then this should fail because running urls during
2139      // shutdown will very likely fail and potentially hang.
2140      nsCOMPtr<nsIMsgAccountManager> accountMgr = do_GetService(NS_MSGACCOUNTMANAGER_CONTRACTID, &rv);
2141      NS_ENSURE_SUCCESS(rv, rv);
2142      bool shuttingDown = false;
2143      (void) accountMgr->GetShutdownInProgress(&shuttingDown);
2144      if (shuttingDown && imapAction != nsIImapUrl::nsImapExpungeFolder &&
2145          imapAction != nsIImapUrl::nsImapDeleteAllMsgs &&
2146          imapAction != nsIImapUrl::nsImapDeleteFolder)
2147        return NS_ERROR_FAILURE;
2148
2149      // if we're running a select or delete all, do a noop first.
2150      // this should really be in the connection cache code when we know
2151      // we're pulling out a selected state connection, but maybe we
2152      // can get away with this.
2153      m_needNoop = (imapAction == nsIImapUrl::nsImapSelectFolder || imapAction == nsIImapUrl::nsImapDeleteAllMsgs);
2154
2155      // We now have a url to run so signal the monitor for url ready to be processed...
2156      ReentrantMonitorAutoEnter urlReadyMon(m_urlReadyToRunMonitor);
2157      m_nextUrlReadyToRun = true;
2158      urlReadyMon.Notify();
2159
2160    } // if we have an imap url and a transport
2161    else
2162      NS_ASSERTION(false, "missing channel or running url");
2163
2164  } // if we received a url!
2165
    CID 1137544 (#1 of 1): Uninitialized scalar variable (UNINIT)3. uninit_use: Using uninitialized value rv.
2166  return rv;
2167}

Observation:

There is a path where |rv| is not set before being used as return value. 

I think we should return an error if aURL is null.
Otherwise, at the end of the function, I think we should return NS_OK if we succeeded inside if (aURL) { }.