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

Status: NEW

(Core :: XUL, defect, P5)

Description

[Mats Palmgren (inactive)]

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());

Comment 1

[Mats Palmgren (inactive)]

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

Comment 2

[Firefox Bug Husbandry Bot]

https://wiki.mozilla.org/Bug_Triage/Projects/Bug_Handling/Bug_Husbandry#Move_fix-optionals

Comment 3

[Ahmad Al Saidi]

Hello,

I am new here. I would like to work on this bug.