Hi Ard, This is my mistake. I usually check the Author field when adding Rb/Ab tags because I have seen lots of cases where the Author field needs updating as well. I forgot to do this step in this one.
I agree a CI check against the Author field being a valid email address and not one of the mailing addresses would be valuable. Perhaps in PatchCheck.py? Mike > -----Original Message----- > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ard > Biesheuvel > Sent: Thursday, July 20, 2023 9:42 AM > To: devel@edk2.groups.io; Kinney, Michael D > <michael.d.kin...@intel.com>; Leif Lindholm <quic_llind...@quicinc.com>; > Demeter, Miki <miki.deme...@intel.com> > Cc: Kasbekar, Saloni <saloni.kasbe...@intel.com>; Nickle Wang > <nick...@nvidia.com>; Clark-williams, Zachary <zachary.clark- > willi...@intel.com>; Abner Chang <abner.ch...@amd.com>; Igor Kulchytskyy > <ig...@ami.com>; Nick Ramirez <nrami...@nvidia.com> > Subject: Re: [edk2-devel] [PATCH 2/2] NetworkPkg/HttpDxe: fix driver > binding start issue. > > On Thu, 20 Jul 2023 at 03:41, Michael D Kinney > <michael.d.kin...@intel.com> wrote: > > > > Merged: https://github.com/tianocore/edk2/pull/4666 > > > > > This commit is now merged as > > commit 6510dcf6f71adbe282bff0ba2b236f1d074f819f > Author: devel@edk2.groups.io <devel@edk2.groups.io> > AuthorDate: Fri Feb 10 04:34:03 2023 -0800 > Commit: mergify[bot] > <37929162+mergify[bot]@users.noreply.github.com> > CommitDate: Thu Jul 20 01:41:02 2023 +0000 > > which is a bit unfortunate. > > Given how pedantic our pre-merge CI is, could we perhaps also add a > check that the author field is sane? I don't think matching it to the > signoff makes sense in all cases so we could just check that it isn't > set to this particular value. (I could contribute code that I did not > author, and my signoff would amount to a statement on my part that > distributing the code as part of EDK2 complies with the code's license > terms and those of EDK2 itself) Alternatively, we might have a PR flag > that allows the maintainer to override this check manually if the > author field does not match the signoff, and match it strictly to the > [first] signoff line in other cases. > > As for the committer field: it is quite unfortunate that we do not > keep track of who merged which PR. Could we do something about that? > The mergify bot is not a member of the project or a Tianocore > maintainer so it is not accountable, and its identity should not > obfuscate the identity of the person who decided to merge the PR. > > > > > > > > -----Original Message----- > > > From: Kinney, Michael D <michael.d.kin...@intel.com> > > > Sent: Wednesday, July 19, 2023 5:27 PM > > > To: Kasbekar, Saloni <saloni.kasbe...@intel.com>; Nickle Wang > > > <nick...@nvidia.com>; devel@edk2.groups.io; Clark-williams, Zachary > > > <zachary.clark-willi...@intel.com> > > > Cc: Abner Chang <abner.ch...@amd.com>; Igor Kulchytskyy > <ig...@ami.com>; > > > Nick Ramirez <nrami...@nvidia.com>; Kinney, Michael D > > > <michael.d.kin...@intel.com> > > > Subject: RE: [edk2-devel] [PATCH 2/2] NetworkPkg/HttpDxe: fix driver > > > binding start issue. > > > > > > Acked-by: Michael D Kinney <michael.d.kin...@intel.com> > > > > > > > -----Original Message----- > > > > From: Kasbekar, Saloni <saloni.kasbe...@intel.com> > > > > Sent: Thursday, July 13, 2023 9:13 AM > > > > To: Nickle Wang <nick...@nvidia.com>; devel@edk2.groups.io; Clark- > > > > williams, Zachary <zachary.clark-willi...@intel.com>; Kinney, > Michael > > > D > > > > <michael.d.kin...@intel.com> > > > > Cc: Abner Chang <abner.ch...@amd.com>; Igor Kulchytskyy > > > <ig...@ami.com>; > > > > Nick Ramirez <nrami...@nvidia.com> > > > > Subject: RE: [edk2-devel] [PATCH 2/2] NetworkPkg/HttpDxe: fix > driver > > > > binding start issue. > > > > > > > > Mike, > > > > > > > > Would you be able to help us merge the patch? > > > > > > > > Thanks, > > > > Saloni > > > > > > > > -----Original Message----- > > > > From: Nickle Wang <nick...@nvidia.com> > > > > Sent: Wednesday, July 12, 2023 11:54 PM > > > > To: devel@edk2.groups.io; Kasbekar, Saloni > > > <saloni.kasbe...@intel.com>; > > > > Clark-williams, Zachary <zachary.clark-willi...@intel.com> > > > > Cc: Abner Chang <abner.ch...@amd.com>; Igor Kulchytskyy > > > <ig...@ami.com>; > > > > Nick Ramirez <nrami...@nvidia.com> > > > > Subject: RE: [edk2-devel] [PATCH 2/2] NetworkPkg/HttpDxe: fix > driver > > > > binding start issue. > > > > > > > > Hi Saloni, > > > > > > > > Could you please help to merge this fix since there is no > objection > > > > during past weeks? > > > > > > > > Thanks, > > > > Nickle > > > > > > > > > -----Original Message----- > > > > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of > > > Saloni > > > > > Kasbekar via groups.io > > > > > Sent: Friday, June 30, 2023 6:28 AM > > > > > To: Nickle Wang <nick...@nvidia.com>; devel@edk2.groups.io > > > > > Cc: Maciej Rabeda <maciej.rab...@linux.intel.com>; Siyuan Fu > > > > > <siyuan...@intel.com>; Abner Chang <abner.ch...@amd.com>; Igor > > > > > Kulchytskyy <ig...@ami.com>; Nick Ramirez <nrami...@nvidia.com> > > > > > Subject: Re: [edk2-devel] [PATCH 2/2] NetworkPkg/HttpDxe: fix > driver > > > > > binding start issue. > > > > > > > > > > External email: Use caution opening links or attachments > > > > > > > > > > > > > > > Hi Nickle, > > > > > > > > > > That makes sense. Thanks for the clarification. > > > > > > > > > > Reviewed-by: Saloni Kasbekar <saloni.kasbe...@intel.com> > > > > > > > > > > Thanks, > > > > > Saloni > > > > > > > > > > -----Original Message----- > > > > > From: Nickle Wang <nick...@nvidia.com> > > > > > Sent: Wednesday, June 28, 2023 3:30 PM > > > > > To: Kasbekar, Saloni <saloni.kasbe...@intel.com>; > > > devel@edk2.groups.io > > > > > Cc: Maciej Rabeda <maciej.rab...@linux.intel.com>; Siyuan Fu > > > > > <siyuan...@intel.com>; Abner Chang <abner.ch...@amd.com>; Igor > > > > > Kulchytskyy <ig...@ami.com>; Nick Ramirez <nrami...@nvidia.com> > > > > > Subject: RE: [edk2-devel] [PATCH 2/2] NetworkPkg/HttpDxe: fix > driver > > > > > binding start issue. > > > > > > > > > > Hi Saloni, > > > > > > > > > > Thanks for your review. > > > > > > > > > > When uninstall fails, per UEFI specification, the protocol will > be > > > > > installed again and will be visible to UEFI drivers. > > > > > > > > > > Page 190, UEFI spec. 2.10: > > > > > "If any errors are generated while the protocol interfaces are > being > > > > > uninstalled, then the protocols uninstalled prior to the error > will > > > be > > > > > reinstalled with the boot service > > > > > EFI_BOOT_SERVICES.InstallProtocolInterface() and the status code > > > > > EFI_INVALID_PARAMETER is returned." > > > > > > > > > > In this case, if we do FreePool while driver still can locate > > > > > gEfiHttpServiceBindingProtocolGuid. Driver will access to the > memory > > > > > that is released to system. Memory issue may happen. > > > > > > > > > > Regards, > > > > > Nickle > > > > > > > > > > > -----Original Message----- > > > > > > From: Kasbekar, Saloni <saloni.kasbe...@intel.com> > > > > > > Sent: Thursday, June 29, 2023 3:07 AM > > > > > > To: devel@edk2.groups.io; Nickle Wang <nick...@nvidia.com> > > > > > > Cc: Maciej Rabeda <maciej.rab...@linux.intel.com>; Siyuan Fu > > > > > > <siyuan...@intel.com>; Abner Chang <abner.ch...@amd.com>; Igor > > > > > > Kulchytskyy <ig...@ami.com>; Nick Ramirez > <nrami...@nvidia.com> > > > > > > Subject: RE: [edk2-devel] [PATCH 2/2] NetworkPkg/HttpDxe: fix > > > driver > > > > > > binding start issue. > > > > > > > > > > > > External email: Use caution opening links or attachments > > > > > > > > > > > > > > > > > > Hi Nickle, > > > > > > > > > > > > We would want to do the FreePool even if the Uninstall fails > (like > > > > > > in the case where we failed to install the multiple protocol > > > > > > interfaces and then went to ON_ERROR). Do you think it's > better if > > > > > > we change it to - > > > > > > > > > > > > if (HttpService != NULL) { > > > > > > HttpCleanService (HttpService, UsingIpv6); > > > > > > Status = gBS->UninstallMultipleProtocolInterfaces ( > > > > > > &ControllerHandle, > > > > > > &gEfiHttpServiceBindingProtocolGuid, > > > > > > &HttpService->ServiceBinding, > > > > > > NULL > > > > > > ); > > > > > > if ((HttpService->Tcp4ChildHandle == NULL) && > > > > > > (HttpService->Tcp6ChildHandle == NULL)) { > > > > > > FreePool (HttpService); > > > > > > } > > > > > > } > > > > > > > > > > > > Thanks, > > > > > > Saloni > > > > > > > > > > > > -----Original Message----- > > > > > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of > > > > > > Nickle Wang via groups.io > > > > > > Sent: Tuesday, June 27, 2023 5:56 PM > > > > > > To: devel@edk2.groups.io; Nickle Wang <nick...@nvidia.com> > > > > > > Cc: Maciej Rabeda <maciej.rab...@linux.intel.com>; Siyuan Fu > > > > > > <siyuan...@intel.com>; Abner Chang <abner.ch...@amd.com>; Igor > > > > > > Kulchytskyy <ig...@ami.com>; Nick Ramirez > <nrami...@nvidia.com> > > > > > > Subject: Re: [edk2-devel] [PATCH 2/2] NetworkPkg/HttpDxe: fix > > > driver > > > > > > binding start issue. > > > > > > > > > > > > May I know if someone can help to review this patch? > > > > > > > > > > > > Thanks, > > > > > > Nickle > > > > > > > > > > > > > -----Original Message----- > > > > > > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf > Of > > > > > > > Nickle Wang via groups.io > > > > > > > Sent: Friday, February 10, 2023 8:34 PM > > > > > > > To: devel@edk2.groups.io > > > > > > > Cc: Maciej Rabeda <maciej.rab...@linux.intel.com>; Siyuan Fu > > > > > > > <siyuan...@intel.com>; Abner Chang <abner.ch...@amd.com>; > Igor > > > > > > > Kulchytskyy <ig...@ami.com>; Nick Ramirez > <nrami...@nvidia.com> > > > > > > > Subject: [edk2-devel] [PATCH 2/2] NetworkPkg/HttpDxe: fix > driver > > > > > > > binding start issue. > > > > > > > > > > > > > > External email: Use caution opening links or attachments > > > > > > > > > > > > > > > > > > > > > When failure happens in HttpDxeStart, the error handling > code > > > > > > > release the memory buffer but it does not uninstall HTTP > service > > > > > > > bindnig protocol. As the result, application can still > locate > > > this > > > > > > > protocol and invoke service binding fucntions in released > memory > > > > pool. > > > > > > > > > > > > > > Signed-off-by: Nickle Wang <nick...@nvidia.com> > > > > > > > Cc: Maciej Rabeda <maciej.rab...@linux.intel.com> > > > > > > > Cc: Siyuan Fu <siyuan...@intel.com> > > > > > > > Cc: Abner Chang <abner.ch...@amd.com> > > > > > > > Cc: Igor Kulchytskyy <ig...@ami.com> > > > > > > > Cc: Nick Ramirez <nrami...@nvidia.com> > > > > > > > --- > > > > > > > NetworkPkg/HttpDxe/HttpDriver.c | 13 +++++++++++-- > > > > > > > 1 file changed, 11 insertions(+), 2 deletions(-) > > > > > > > > > > > > > > diff --git a/NetworkPkg/HttpDxe/HttpDriver.c > > > > > > > b/NetworkPkg/HttpDxe/HttpDriver.c index > 5d918d3c4d..f6d1263cad > > > > > > > 100644 > > > > > > > --- a/NetworkPkg/HttpDxe/HttpDriver.c > > > > > > > +++ b/NetworkPkg/HttpDxe/HttpDriver.c > > > > > > > @@ -3,6 +3,7 @@ > > > > > > > > > > > > > > Copyright (c) 2015 - 2018, Intel Corporation. All rights > > > > reserved.<BR> > > > > > > > (C) Copyright 2016 Hewlett Packard Enterprise Development > > > > > > > LP<BR> > > > > > > > + Copyright (c) 2023, NVIDIA CORPORATION & AFFILIATES. All > > > rights > > > > > reserved. > > > > > > > > > > > > > > SPDX-License-Identifier: BSD-2-Clause-Patent > > > > > > > > > > > > > > @@ -464,8 +465,16 @@ ON_ERROR: > > > > > > > > > > > > > > if (HttpService != NULL) { > > > > > > > HttpCleanService (HttpService, UsingIpv6); > > > > > > > - if ((HttpService->Tcp4ChildHandle == NULL) && > (HttpService- > > > > > > > >Tcp6ChildHandle == NULL)) { > > > > > > > - FreePool (HttpService); > > > > > > > + Status = gBS->UninstallMultipleProtocolInterfaces ( > > > > > > > + &ControllerHandle, > > > > > > > + &gEfiHttpServiceBindingProtocolGuid, > > > > > > > + &HttpService->ServiceBinding, > > > > > > > + NULL > > > > > > > + ); > > > > > > > + if (!EFI_ERROR (Status)) { > > > > > > > + if ((HttpService->Tcp4ChildHandle == NULL) && > > > (HttpService- > > > > > > > >Tcp6ChildHandle == NULL)) { > > > > > > > + FreePool (HttpService); > > > > > > > + } > > > > > > > } > > > > > > > } > > > > > > > > > > > > > > -- > > > > > > > 2.39.1.windows.1 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#107107): https://edk2.groups.io/g/devel/message/107107 Mute This Topic: https://groups.io/mt/99821789/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
