1361609 - Coverity report: nsCSSFrameConstructor::​BeginBuildingScrollFrame : All paths that lead to this null pointer comparison already dereference the pointer earlier

Status: RESOLVED DUPLICATE of bug 1529619

(Core :: Layout, enhancement, P3)

Description

[Mats Palmgren (inactive)]

Coverity CID 1221199 Dereference before null check

There may be a null pointer dereference, or else the comparison against null is unnecessary.

In nsCSSFrameConstructor::​BeginBuildingScrollFrame(nsFrameConstructorState &, nsIContent *, nsStyleContext *, nsContainerFrame *, nsIAtom *, bool, nsContainerFrame *&): All paths that lead to this null pointer comparison already dereference the pointer earlier 




4614  aNewFrame = gfxScrollFrame;
4615  gfxScrollFrame->AddStateBits(NS_FRAME_OWNS_ANON_BOXES);
4616
4617  // we used the style that was passed in. So resolve another one.
4618  StyleSetHandle styleSet = mPresShell->StyleSet();
4619  RefPtr<nsStyleContext> scrolledChildStyle =
4620    styleSet->ResolveInheritingAnonymousBoxStyle(aScrolledPseudo, contentStyle);
4621
    CID 1221199 (#1 of 1): Dereference before null check (REVERSE_INULL)check_after_deref: Null-checking gfxScrollFrame suggests that it may be null, but it has already been dereferenced on all paths leading to the check.
4622  if (gfxScrollFrame) {
4623     gfxScrollFrame->SetInitialChildList(kPrincipalList, anonymousItems);
4624  }

Comment 1

[Mats Palmgren (inactive)]

It seems we should just remove this null-check:
http://searchfox.org/mozilla-central/rev/abe68d5dad139e376d5521ca1d4b7892e1e7f1ba/layout/base/nsCSSFrameConstructor.cpp#4615,4622

Comment 2

[Shrey Gupta]

Hello,I would like to work on this bug

Comment 3

[ailana.askar]

Currently this bug seems to not appear, because null-check isn't occurring anymore.

Comment 4

[Mike Conley (:mconley) (:⚙️)]

I concur with ailana. Here's the current code in question: https://searchfox.org/mozilla-central/rev/1ca8ea11406642df4a2c6f81f21d683817af568d/layout/base/nsCSSFrameConstructor.cpp#4174,4206

I think we can probably close this as INVALID, wdyt dholbert?

Comment 5

[Daniel Holbert [:dholbert]]

Yup - looks like we fixed this in bug 1529619 which was effectively a dupe of this one.

Given that we fixed it over there, I'll dupe this to bug 1529619.