coverity report: In nsXULPopupManager::​ShowMenu(nsIContent *, bool, bool): Pointer is checked against null but then dereferenced anyway

NEW
Unassigned

Status

()

Core
XUL
P5
trivial
7 months ago
2 months ago

People

(Reporter: mats, Unassigned)

Tracking

({coverity, good-first-bug})

Trunk
coverity, good-first-bug
Points:
---

Firefox Tracking Flags

(firefox56 wontfix, firefox57 wontfix, firefox58 fix-optional)

Details

(Reporter)

Description

7 months ago
Coverity CID 748844 Dereference after null check

Either the check against null is unnecessary, or there may be a null pointer dereference.

In nsXULPopupManager::​ShowMenu(nsIContent *, bool, bool): Pointer is checked against null but then dereferenced anyway


697nsXULPopupManager::ShowMenu(nsIContent *aMenu,
698                            bool aSelectFirstItem,
699                            bool aAsynchronous)
700{
701  // generate any template content first. Otherwise, the menupopup may not
702  // have been created yet.
   1. Condition aMenu, taking false branch.
   2. var_compare_op: Comparing aMenu to null implies that aMenu might be null.
703  if (aMenu) {
704    nsIContent* element = aMenu;
705    do {
706      RefPtr<nsXULElement> xulelem = nsXULElement::FromContent(element);
707      if (xulelem) {
708        nsCOMPtr<nsIXULTemplateBuilder> builder = xulelem->GetBuilder();
709        if (builder) {
710          builder->CreateContents(aMenu, true);
711          break;
712        }
713      }
714      element = element->GetParent();
715    } while (element);
716  }
717
   CID 748844 (#1 of 1): Dereference after null check (FORWARD_NULL)3. var_deref_model: Passing null pointer aMenu to GetPrimaryFrame, which dereferences it. [show details]
718  nsMenuFrame* menuFrame = do_QueryFrame(aMenu->GetPrimaryFrame());
(Reporter)

Comment 1

7 months ago
We can probably just MOZ_ASSERT(aMenu) at the top and remove the "if (aMenu)" check.

http://searchfox.org/mozilla-central/rev/abe68d5dad139e376d5521ca1d4b7892e1e7f1ba/layout/xul/nsXULPopupManager.cpp#697
Keywords: good-first-bug

Updated

2 months ago
status-firefox56: --- → wontfix
status-firefox57: --- → wontfix
status-firefox58: --- → fix-optional
Priority: -- → P5
You need to log in before you can comment on or make changes to this bug.