Re: [PATCH v1] MdeModulePkg/DxeCore: Please static checker for false report

["Michael D Kinney" <michael.d.kinney@intel.com>]

Ard,

This seems to be a common limitation seen in some
static analyzers.  We have not found a workaround
that does not involve code changes to quiet the 
false positives.

For this specific case, I think the code change I
suggest is correct.  

Best regards,

Mike

> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: Monday, April 22, 2019 2:26 PM
> To: Kinney, Michael D <michael.d.kinney@intel.com>
> Cc: Wu, Hao A <hao.a.wu@intel.com>;
> devel@edk2.groups.io; Gao, Liming
> <liming.gao@intel.com>; Wang, Jian J
> <jian.j.wang@intel.com>
> Subject: Re: [PATCH v1] MdeModulePkg/DxeCore: Please
> static checker for false report
> 
> On Mon, 22 Apr 2019 at 16:41, Kinney, Michael D
> <michael.d.kinney@intel.com> wrote:
> >
> > Hi Hao,
> >
> > I think a cleaner fix to this issues is replace both
> > ASSERT() statements with the following:
> >
> >       if (EFI_ERROR (Status) || Entry->Emulator ==
> NULL) {
> >         FreePool (Entry);
> >         continue;
> >       }
> >
> > We do not expect the emulator protocol to disappear
> between
> > finding the handle and looking up the protocol
> instance,
> > but if it does, the handle can be skipped without
> ASSERT().
> >
> > There are several examples of this style in
> DriverSupport.c.
> >
> > If we want to avoid the extra Allocate/Free in this
> error
> > condition, then a local variable can be added to get
> the
> > emulator protocol instance and only allocate an
> > EMULATOR_ENTRY if the emulator instance is
> successfully
> > found.
> >
> 
> Is there any way we can #define the OUT modifier to
> something the
> static analyzer understands? (Which static analyzer is
> this btw?)
> 
> Surely, we are not the only project dealing with
> pointers that are
> initialized by reference. Adding code to please the
> tools should
> really be the last resort imo.