(coverity) Uninitialized scalar variable: mailnews/local/src/nsLocalMailFolder.cpp: |rv| is not in a path before being returned by |return rv|.

NEW
Unassigned

Status

MailNews Core
Database
a year ago
a year ago

People

(Reporter: ISHIKAWA, Chiaki, Unassigned)

Tracking

(Blocks: 1 bug, {coverity})

Trunk
coverity

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: CID 1137543, URL)

(Reporter)

Description

a year ago
Coverity found this:

There is a path where |rv| is never set before being returned by
|return rv|.


486NS_IMETHODIMP nsMsgLocalMailFolder::CreateStorageIfMissing(nsIUrlListener* aUrlListener)
487{
   1. var_decl: Declaring variable rv without initializer.
488  nsresult rv;
489  nsCOMPtr<nsIMsgFolder> msgParent;
490  GetParent(getter_AddRefs(msgParent));
491
492  // parent is probably not set because *this* was probably created by rdf
493  // and not by folder discovery. So, we have to compute the parent.
   2. Condition !msgParent.operator bool(), taking true branch
494  if (!msgParent)
495  {
496    nsAutoCString folderName(mURI);
497    nsAutoCString uri;
498    int32_t leafPos = folderName.RFindChar('/');
499    nsAutoCString parentName(folderName);
   3. Condition leafPos > 0, taking false branch
500    if (leafPos > 0)
501    {
502      // If there is a hierarchy, there is a parent.
503      // Don't strip off slash if it's the first character
504      parentName.SetLength(leafPos);
505      // get the corresponding RDF resource
506      // RDF will create the folder resource if it doesn't already exist
507      nsCOMPtr<nsIRDFService> rdf = do_GetService("@mozilla.org/rdf/rdf-service;1", &rv);
508      NS_ENSURE_SUCCESS(rv,rv);
509
510      nsCOMPtr<nsIRDFResource> resource;
511      rv = rdf->GetResource(parentName, getter_AddRefs(resource));
512      NS_ENSURE_SUCCESS(rv,rv);
513
514      msgParent = do_QueryInterface(resource, &rv);
515      NS_ENSURE_SUCCESS(rv,rv);
516    }
517  }
518
   4. Condition msgParent.operator bool(), taking false branch
519  if (msgParent)
520  {
521    nsString folderName;
522    GetName(folderName);
523    rv = msgParent->CreateSubfolder(folderName, nullptr);
524    // by definition, this is OK.
525    if (rv == NS_MSG_FOLDER_EXISTS)
526      return NS_OK;
527  }
528
   CID 1137543 (#1 of 1): Uninitialized scalar variable (UNINIT)5. uninit_use: Using uninitialized value rv.
529  return rv;
530}

Observation:

What do we want to return *IF* msgParent is still false on 519, i.e., even after the computation of msgParent above. I think given the check on 
     msgParent = do_QueryInterface(resource, &rv);
515      NS_ENSURE_SUCCESS(rv,rv);

we probably want to return NS_ERROR_UNEXPECTED or something.
I think it would be best to make this intention clear by adding else after 527.

and return rv after 526.
So that would make the code as
519  if (msgParent)
520  {
521    nsString folderName;
522    GetName(folderName);
523    rv = msgParent->CreateSubfolder(folderName, nullptr);
524    // by definition, this is OK.
525    if (rv == NS_MSG_FOLDER_EXISTS)
526      return NS_OK;
           return rv;
527  }
     else
        return NS_ERROR_UNEXPECTED;

(I would set the initial value of |rv| to be false just in case.)
You need to log in before you can comment on or make changes to this bug.