From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=192.55.52.88; helo=mga01.intel.com; envelope-from=hao.a.wu@intel.com; receiver=edk2-devel@lists.01.org Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id B486021F3041A for ; Thu, 28 Sep 2017 17:55:58 -0700 (PDT) Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 28 Sep 2017 17:59:13 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.42,451,1500966000"; d="scan'208";a="156803145" Received: from fmsmsx104.amr.corp.intel.com ([10.18.124.202]) by fmsmga005.fm.intel.com with ESMTP; 28 Sep 2017 17:59:13 -0700 Received: from shsmsx152.ccr.corp.intel.com (10.239.6.52) by fmsmsx104.amr.corp.intel.com (10.18.124.202) with Microsoft SMTP Server (TLS) id 14.3.319.2; Thu, 28 Sep 2017 17:59:13 -0700 Received: from shsmsx101.ccr.corp.intel.com ([169.254.1.159]) by SHSMSX152.ccr.corp.intel.com ([169.254.6.93]) with mapi id 14.03.0319.002; Fri, 29 Sep 2017 08:59:03 +0800 From: "Wu, Hao A" To: Udit Kumar , "edk2-devel@lists.01.org" CC: "Kinney, Michael D" , "Yao, Jiewen" , "Zeng, Star" Thread-Topic: [edk2] [PATCH] MdeModulePkg/DxeCore: Add comments for the ASSERT to check NULL ptr Thread-Index: AQHTOCefpqP79GAZ4UySkALfx3NbJKLJgZcAgAGIRuA= Date: Fri, 29 Sep 2017 00:59:03 +0000 Message-ID: References: <20170928070039.632-1-hao.a.wu@intel.com> In-Reply-To: Accept-Language: zh-CN, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [PATCH] MdeModulePkg/DxeCore: Add comments for the ASSERT to check NULL ptr X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 29 Sep 2017 00:55:58 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ud= it > Kumar > Sent: Thursday, September 28, 2017 5:28 PM > To: Wu, Hao A; edk2-devel@lists.01.org > Cc: Kinney, Michael D; Yao, Jiewen; Zeng, Star > Subject: Re: [edk2] [PATCH] MdeModulePkg/DxeCore: Add comments for the > ASSERT to check NULL ptr >=20 >=20 >=20 > > -----Original Message----- > > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of = Hao > > Wu > > Sent: Thursday, September 28, 2017 12:31 PM > > To: edk2-devel@lists.01.org > > Cc: Hao Wu ; Michael D Kinney > > ; Jiewen Yao ; Star > Zeng > > > > Subject: [edk2] [PATCH] MdeModulePkg/DxeCore: Add comments for the > > ASSERT to check NULL ptr > > > > Commit 8932679df5be046feba30fae80776c5815232a08 adds an ASSERT for > > checking NULL pointer dereference. > > > > This commit adds comments to clarify the reason for using ASSERT as the > check. > > > > Cc: Star Zeng > > Cc: Michael D Kinney > > Cc: Jiewen Yao > > Contributed-under: TianoCore Contribution Agreement 1.1 > > Signed-off-by: Hao Wu > > --- > > MdeModulePkg/Core/Dxe/Hand/Handle.c | 7 ++++++- > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > diff --git a/MdeModulePkg/Core/Dxe/Hand/Handle.c > > b/MdeModulePkg/Core/Dxe/Hand/Handle.c > > index 2db441725c..344ff1fe02 100644 > > --- a/MdeModulePkg/Core/Dxe/Hand/Handle.c > > +++ b/MdeModulePkg/Core/Dxe/Hand/Handle.c > > @@ -1175,10 +1175,15 @@ Done: > > // > > if (!EFI_ERROR (Status) || Status =3D=3D EFI_ALREADY_STARTED) { > > // > > + // According to above logic, if 'Prot' is NULL, then the 'Status= ' must be > > + // EFI_UNSUPPORTED. Here the 'Status' is not EFI_UNSUPPORTED, so > 'Prot' > > + // must be not NULL. > > + // > > + ASSERT (Prot !=3D NULL); > > + // >=20 > I think , we should take care of no debug environment here > If MDEPKG_NDEBUG is not defined and Prot is NULL then > shouldn't we return error ? Hi Udit Kumar, As mentioned in another feedback for this patch from Mike, the ASSERT here is added duet to a false positive report from static analysis. The code logic actually ensures that 'Prot' will not be NULL within the 'if' statement: "if (!EFI_ERROR (Status) || Status =3D=3D EFI_ALREADY_STARTED) {" I will refine the patch to add more comments for the ASSERT used here so that later if there's improvement for the static analysis, we can locate and remove this ASSERT by searching keywords in the comments. Best Regards, Hao Wu >=20 > > // EFI_ALREADY_STARTED is not an error for bus driver. > > // Return the corresponding protocol interface. > > // > > - ASSERT (Prot !=3D NULL); > > *Interface =3D Prot->Interface; > > } else if (Status =3D=3D EFI_UNSUPPORTED) { > > // > > -- > > 2.12.0.windows.1 > > > > _______________________________________________ > > edk2-devel mailing list > > edk2-devel@lists.01.org > > https://lists.01.org/mailman/listinfo/edk2-devel > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel