From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 192.55.52.136, mailfrom: dandan.bi@intel.com) Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) by groups.io with SMTP; Wed, 11 Sep 2019 18:59:56 -0700 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga001.jf.intel.com ([10.7.209.18]) by fmsmga106.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 11 Sep 2019 18:59:56 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.64,492,1559545200"; d="scan'208";a="268936680" Received: from fmsmsx103.amr.corp.intel.com ([10.18.124.201]) by orsmga001.jf.intel.com with ESMTP; 11 Sep 2019 18:59:55 -0700 Received: from shsmsx152.ccr.corp.intel.com (10.239.6.52) by FMSMSX103.amr.corp.intel.com (10.18.124.201) with Microsoft SMTP Server (TLS) id 14.3.439.0; Wed, 11 Sep 2019 18:59:55 -0700 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.32]) by SHSMSX152.ccr.corp.intel.com ([169.254.6.132]) with mapi id 14.03.0439.000; Thu, 12 Sep 2019 09:59:52 +0800 From: "Dandan Bi" To: Heinrich Schuchardt , EDK II Development CC: "Wu, Hao A" , "Wang, Jian J" , "Gao, Liming" , "Zeng, Star" , "Yao, Jiewen" , Laszlo Ersek , "Jin, Eric" , Supreeth Venkatesh , Stephano Cetola Subject: Re: [edk2-core] [PATCH v2 1/1] MdeModulePkg: Make retval in UninstallMultipleProtocol follow Spec Thread-Topic: [edk2-core] [PATCH v2 1/1] MdeModulePkg: Make retval in UninstallMultipleProtocol follow Spec Thread-Index: AQHVZ6+JbVMLxiHvyUGUevP53rSS/6cnRspw Date: Thu, 12 Sep 2019 01:59:51 +0000 Message-ID: <3C0D5C461C9E904E8F62152F6274C0BB40C5AC3A@SHSMSX104.ccr.corp.intel.com> References: <20190910081219.3680-1-xypron.glpk@gmx.de> In-Reply-To: <20190910081219.3680-1-xypron.glpk@gmx.de> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Return-Path: dandan.bi@intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Hi , Thanks for the update.=20 I don't know how you send this patch, the format and subject seems a little= different from V1 patch. Anyway it doesn't block my review of this patch. I have two minor comments for the function comments: 1. Per EDKII C Coding Spec, @retval for each unique return value, @return f= or a function's return values when those values aren't easily described by = @retval commands. So here I think we should use @retval instead of @return. 2. Per my understanding, the protocol interfaces uninstalled prior to the e= rror seems not to be reinstalled in reverse order of uninstalling, the rein= stallation seems in the same order of previous uninstalling (start from the= first pair after the handle to next...). Please help double check this iss= ue and update the comments if needed . Since these are the comments update and with these addressed, Reviewed-by:= Dandan Bi Thanks, Dandan > -----Original Message----- > From: Heinrich Schuchardt [mailto:xypron.glpk@gmx.de] > Sent: Tuesday, September 10, 2019 4:12 PM > To: EDK II Development ; Bi, Dandan > > Cc: Wu, Hao A ; Wang, Jian J ; > Gao, Liming ; Zeng, Star ; Yao= , > Jiewen ; Laszlo Ersek ; Jin, Eri= c > ; Supreeth Venkatesh ; > Stephano Cetola ; Heinrich Schuchardt > > Subject: [edk2-core] [PATCH v2 1/1] MdeModulePkg: Make retval in > UninstallMultipleProtocol follow Spec >=20 > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D1869 >=20 > The UEFI spec requires that if any error occurs in > UninstallMultipleProtocolInterfaces(), EFI_INVALID_PARAMETER is returned > and not the return code of UninstallProtocolInterface(). >=20 > Signed-off-by: Heinrich Schuchardt > --- > v2 > Adjust the subject line. > Adjust the function comments to clarify the behavior. > This replaces https://edk2.groups.io/g/devel/message/46974 > --- > MdeModulePkg/Core/Dxe/Hand/Handle.c | 21 +++++++++++++++------ > 1 file changed, 15 insertions(+), 6 deletions(-) >=20 > diff --git a/MdeModulePkg/Core/Dxe/Hand/Handle.c > b/MdeModulePkg/Core/Dxe/Hand/Handle.c > index b2721b3ab2..719ba98261 100644 > --- a/MdeModulePkg/Core/Dxe/Hand/Handle.c > +++ b/MdeModulePkg/Core/Dxe/Hand/Handle.c > @@ -802,20 +802,28 @@ Done: > - /** Uninstalls a list of protocol interface in the boot services > environment.- This function calls UnisatllProtocolInterface() in a loop.= This > is+ This function calls UninstallProtocolInterface() in a loop. This is = basically a > lib function to save space. - @param Handle The handle = to uninstall > the protocol+ If any errors are generated while the protocol interfaces = are > being+ uninstalled, then the protocol interfaces uninstalled prior to th= e error > will+ be reinstalled in reverse order of uninstalling and > EFI_INVALID_PARAMETER is+ returned.++ @param Handle Th= e > handle to uninstall the protocol interfaces+ = from. @param ... > EFI_GUID followed by protocol instance. A NULL- = terminates > the list. The pairs are the+ terminates = the list. The pairs are > the arguments to UninstallProtocolInterf= ace(). All > the protocols are added to Handle. - @return Status code-+ @return > EFI_SUCCESS if all protocol interfaces where uninstalled.+ @r= eturn > EFI_INVALID_PARAMETER if any protocol interface could not be+ > uninstalled and an attempt was made to+ r= einstall previously > uninstalled protocol+ interfaces. **/ EFI= _STATUS EFIAPI@@ - > 864,6 +872,7 @@ CoreUninstallMultipleProtocolInterfaces ( > CoreInstallProtocolInterface (&Handle, Protocol, EFI_NATIVE_INTERF= ACE, > Interface); } VA_END (Args);+ Status =3D EFI_INVALID_PARAMETER= ; } > return Status;-- > 2.20.1