From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=pass header.i=@gmx.net header.s=badeba3b8450 header.b=YuikFw4j; spf=pass (domain: gmx.de, ip: 212.227.17.21, mailfrom: xypron.glpk@gmx.de) Received: from mout.gmx.net (mout.gmx.net [212.227.17.21]) by groups.io with SMTP; Wed, 11 Sep 2019 23:18:28 -0700 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=gmx.net; s=badeba3b8450; t=1568269090; bh=RvfiYcJ4Wq0lEW1xmYH7h7K6noSl79QxyFf92VuRKoQ=; h=X-UI-Sender-Class:Subject:To:Cc:References:From:Date:In-Reply-To; b=YuikFw4jOGd2C5jMEkTqr+/7QwmDhyoYTJCkIHDfSc5beDWtaHVYTTxEjNELGAMVJ bgDslWFbZsmRWBnoqeMpIY6FnjpGI0Tv3xz/WhyvRw3XimnqwSlx/0Q3b30eH6+KjE c82UO/yaoRmxZcgq/uQQJ9d2OG1kfkFWEJ9/ycvg= X-UI-Sender-Class: 01bb95c1-4bf8-414a-932a-4f6e2808ef9c Received: from [192.168.123.51] ([84.118.159.3]) by mail.gmx.com (mrgmx102 [212.227.17.168]) with ESMTPSA (Nemesis) id 0MRSeC-1hfbUg23A0-00ShP3; Thu, 12 Sep 2019 08:18:10 +0200 Subject: Re: [edk2-core] [PATCH v2 1/1] MdeModulePkg: Make retval in UninstallMultipleProtocol follow Spec To: "Bi, Dandan" , EDK II Development Cc: "Wu, Hao A" , "Wang, Jian J" , "Gao, Liming" , "Zeng, Star" , "Yao, Jiewen" , Laszlo Ersek , "Jin, Eric" , Supreeth Venkatesh , Stephano Cetola References: <20190910081219.3680-1-xypron.glpk@gmx.de> <3C0D5C461C9E904E8F62152F6274C0BB40C5AC3A@SHSMSX104.ccr.corp.intel.com> From: "Heinrich Schuchardt" Message-ID: <21185bdc-9279-a339-b7f6-1cba7ff71ae8@gmx.de> Date: Thu, 12 Sep 2019 08:18:07 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0 MIME-Version: 1.0 In-Reply-To: <3C0D5C461C9E904E8F62152F6274C0BB40C5AC3A@SHSMSX104.ccr.corp.intel.com> X-Provags-ID: V03:K1:5oIegyRNhgFwr5mRMSyIv85tV60g0P+wm6FeFMM6p5jHPHjGBYc KWzu3mhTGmf/iApx40o/8WQFUtqKr0sh2YiK3VZo3i27yNUyQkkpDvNA/apyFp1GsDOcq+Q mSpLvVorEcAJW4Do42+o9lu99OVVVSpPhJIaBxnNx53Q8VuKQmU4eCY2qr/Y04FZJBE4YT+ ivz0uXsACqHr6pB9xcM/w== X-Spam-Flag: NO X-UI-Out-Filterresults: notjunk:1;V03:K0:AEbLw2BQYRw=:ABxQTJefVHHWI6jf1ZLpd7 ElmYIoT2UFuws9ztOnwFAZtMf4PUnMZ36tPY3ioy6bXdMc/MkmmnYAAFj1VFEU13bIdPHEJ90 t31vwJxo/f39HRHJ3Cr48pjA40tBgTmK8tlb+TV0KM+UJ/DnVBSBeBOaaJ8WniawBtkUs7hO2 vGTE0bY9K+SJvPGfxd32BmsdcrQ1H2RQODeeFLPbov5maiQQjsroqpdP0q0I4f6Zgc1csPyvN Id0FXy8l8LD4Ji293nzOKOmWqznGxTxXnxgcgXnGacU0v4tQfxy3kt/tc4Ua8y+2TDJVDHSFd CvIUDVLGp9yPpUucqO9xfidBoA2anGf8jtzDYBVVubo4+REjDLqO+I2U3FF4p6sdO67YuLlpP egxne9rodxwZwDLgkGE6NR4UxR6IrX5Z82lDm9hbCt4Mws+v9YTLBqsp1hbempvh1JJxwu0FW OB48Adk0wWCdK1+sXW9I+yvrRpwAtljJolcGjQYavojIBFAN4GsedkfNgMIr3EgoN83W/JJ5X xrekXMevD7uTuDQ611xhSv5MHey9GtxgHRnzHe/dTGNdyadvzoEvpQxP/K3x9EQ7yXYzb8k5J QjTaFJgXrKJkVKJmWfNYPrRfszJUc4KbeEjPSLFEicEmJJ6UeshWziKXfaXCO7y5yxsol2K7V g2tZfKngj/s5fjEKfFm23irHR0OWu3VOY5ztjSkADxy1rLT6qalO9acp0cYkigi2vLhDQonov dpRg9RrR8dh9iZv41HOXYF1dkZO42EtwreSjvYsyobyXBMlusbVHy9NADCiCawUMCyTKW6S5W MxXW/5nAWLhEv6ETkfhQ/KQAzelStPWlnjDjdikQ3puBRYVjFQ5hbGmWdBhhuxGQr6ya5Ul2e q9l1359F2sdufTtqH0JQtfEipOcF+f5dZTBXZgLT3uwQZPwXMPy30PP2zxhnFCm/NcpzzWQF/ aXN1i+wGvWWmdU8Ymj0jQcn6YcgIdt8wU4LddgOCiWti67mzMOtoIH6B22rPIhQXj5mejbsbG 4jtDpNwo1Tvigsee81q9XRO+PvoA7wvI5tVrz2DHbr7TyhhLl6M2NGM5BRRtbu+sCaBsceRdx /CBms64L8NIJi86c7yjOtOrk9q/7DMZPUEgWAk+gxQLWDhqRNdzqHnpHYBvqf3O0Wwife/jhh l3NiwxTCuIhu3dEl0UsXefY5U3u7dEORwAdCq9/rsSMCSSWwn7SYN1Cff8yRmejrLhNEg= Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: quoted-printable On 9/12/19 3:59 AM, Bi, Dandan wrote: > Hi , > > Thanks for the update. > I don't know how you send this patch, the format and subject seems a lit= tle different from V1 patch. Anyway it doesn't block my review of this pat= ch. > > I have two minor comments for the function comments: > 1. Per EDKII C Coding Spec, @retval for each unique return value, @retur= n for a function's return values when those values aren't easily described= by @retval commands. Thanks for pointing me to the coding spec (available at https://github.com/tianocore-docs/Docs/raw/master/Specifications/CCS_2_1_D= raft.pdf). I will correct that. Looking at the rest of the file you will other examples of incorrect use of @return: CoreUnregisterProtocolNotifyEvent. > So here I think we should use @retval instead of @return. > 2. Per my understanding, the protocol interfaces uninstalled prior to th= e error seems not to be reinstalled in reverse order of uninstalling, the = reinstallation seems in the same order of previous uninstalling (start fro= m the first pair after the handle to next...). Please help double check th= is issue and update the comments if needed . You are right. VA_ARG() moves form first to last. Thanks for reviewing. Best regards Heinrich > > 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 ; Y= ao, >> Jiewen ; Laszlo Ersek ; Jin, E= ric >> ; Supreeth Venkatesh ; >> Stephano Cetola ; Heinrich Schuchardt >> >> Subject: [edk2-core] [PATCH v2 1/1] MdeModulePkg: Make retval in >> UninstallMultipleProtocol follow Spec >> >> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D1869 >> >> The UEFI spec requires that if any error occurs in >> UninstallMultipleProtocolInterfaces(), EFI_INVALID_PARAMETER is returne= d >> and not the return code of UninstallProtocolInterface(). >> >> 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(-) >> >> 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 service= s >> environment.- This function calls UnisatllProtocolInterface() in a loo= p. This >> is+ This function calls UninstallProtocolInterface() in a loop. This i= s basically a >> lib function to save space. - @param Handle The handl= e to uninstall >> the protocol+ If any errors are generated while the protocol interface= s are >> being+ uninstalled, then the protocol interfaces uninstalled prior to = the error >> will+ be reinstalled in reverse order of uninstalling and >> EFI_INVALID_PARAMETER is+ returned.++ @param Handle = The >> handle to uninstall the protocol interfaces+ = from. @param ... >> EFI_GUID followed by protocol instance. A NULL- = terminates >> the list. The pairs are the+ terminate= s the list. The pairs are >> the arguments to UninstallProtocolInte= rface(). All >> the protocols are added to Handle. - @return Status code-+ @return >> EFI_SUCCESS if all protocol interfaces where uninstalled.+ = @return >> EFI_INVALID_PARAMETER if any protocol interface could not be+ >> uninstalled and an attempt was made to+ = reinstall previously >> uninstalled protocol+ interfaces. **/ E= FI_STATUS EFIAPI@@ - >> 864,6 +872,7 @@ CoreUninstallMultipleProtocolInterfaces ( >> CoreInstallProtocolInterface (&Handle, Protocol, EFI_NATIVE_INT= ERFACE, >> Interface); } VA_END (Args);+ Status =3D EFI_INVALID_PARAMET= ER; } >> return Status;-- >> 2.20.1 > >