Re: [edk2-devel] [PATCH v2 2/5] MdeModulePkg/Bus/Pci/PciBusDxe: Fix MISSING_BREAK Coverity issues

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

This comment style only works with Coverity.

Other static analysis tools may flag the same issue again.

It is better to update the logic so no static analysis tool will
flag this issue.

Mike

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Laszlo
> Ersek
> Sent: Tuesday, November 7, 2023 8:23 AM
> To: devel@edk2.groups.io; rsingh@ventanamicro.com
> Cc: Ni, Ray <ray.ni@intel.com>; Veeresh Sangolli
> <veeresh.sangolli@dellteam.com>
> Subject: Re: [edk2-devel] [PATCH v2 2/5]
> MdeModulePkg/Bus/Pci/PciBusDxe: Fix MISSING_BREAK Coverity issues
> 
> On 11/7/23 07:19, Ranbir Singh wrote:
> > From: Ranbir Singh <Ranbir.Singh3@Dell.com>
> >
> > The function UpdatePciInfo has switch-case code in which there are
> fall
> > through from case 32: to case 64:. While this is seeemingly
> intentional,
> > it is not evident to any general code reader why there is no break;
> in
> > between. Adding
> >
> >     // No break; here as this is an intentional fallthrough.
> >
> > as comment in between makes it explicit. Incidentally, the comment
> > satisfies Coverity as well.
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4239
> >
> > Cc: Ray Ni <ray.ni@intel.com>
> > Co-authored-by: Veeresh Sangolli <veeresh.sangolli@dellteam.com>
> > Signed-off-by: Ranbir Singh <Ranbir.Singh3@Dell.com>
> > Signed-off-by: Ranbir Singh <rsingh@ventanamicro.com>
> > ---
> >  MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
> b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
> > index 6594b8eae83f..eda97285ee18 100644
> > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
> > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
> > @@ -1428,6 +1428,9 @@ UpdatePciInfo (
> >                switch (Ptr->AddrSpaceGranularity) {
> >                  case 32:
> >                    PciIoDevice->PciBar[BarIndex].BarType =
> PciBarTypeMem32;
> > +                  //
> > +                  // No break; here as this is an intentional fall
> through.
> > +                  //
> >                  case 64:
> >                    PciIoDevice->PciBar[BarIndex].BarTypeFixed =
> TRUE;
> >                    break;
> > @@ -1440,6 +1443,9 @@ UpdatePciInfo (
> >                switch (Ptr->AddrSpaceGranularity) {
> >                  case 32:
> >                    PciIoDevice->PciBar[BarIndex].BarType =
> PciBarTypePMem32;
> > +                  //
> > +                  // No break; here as this is an intentional fall
> through.
> > +                  //
> >                  case 64:
> >                    PciIoDevice->PciBar[BarIndex].BarTypeFixed =
> TRUE;
> >                    break;
> 
> Agree, but the semicolon's placement is awkward. I propose
> 
>   No break here, as this is an intentional fall through.
> 
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> 
> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110870): https://edk2.groups.io/g/devel/message/110870
Mute This Topic: https://groups.io/mt/102438299/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-