Re: [Patch v2] NetworkPkg: UefiPxeBcDxe: Add EXCLUSIVE attribute when opening SNP protocol installed by PXE.

["Wu, Jiaxin" <jiaxin.wu@intel.com>]

> > Subject: [Patch v2] NetworkPkg: UefiPxeBcDxe: Add EXCLUSIVE attribute
> when opening SNP protocol installed by PXE.
> >
> > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1152
> >
> > v2: Sync the same logic to Ipv6 and update code comments.
> >
> > The PXE driver installs a SNP and open this SNP with attribute BY_DRIVER
> > to avoid it being opened by MNP driver, this SNP is also expected not to
> > be opened by other drivers with EXCLUSIVE attribute. In some cases, other
> > drivers may happen to do this by error, and thus cause a system crash.
> > This patch adds EXCLUSIVE attribute when opening SNP in PXE driver, and
> > will reject all OpenProtocol requests by EXCLUSIVE.
> >
> > Cc: Subramanian, Sriram <sriram-s@hpe.com>
> > Cc: Ye Ting <ting.ye@intel.com>
> > Cc: Fu Siyuan <siyuan.fu@intel.com>
> > Cc: Wu Jiaxin <jiaxin.wu@intel.com>
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Wang Fan <fan.wang@intel.com>
> > ---
> >  NetworkPkg/UefiPxeBcDxe/PxeBcDriver.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> Either my edk2-devel subscription is breaking down, or our discipline
> regarding commit pushing is down the drain, recently.
> 
> This patch has been pushed as commit cde5a72d365e. Problems with that
> commit:
> 
> (1) The git authorship is marked as "edk2-devel-bounces@lists.01.org".
> That's *wrong*. This patch was authored by Wang Fan
> <fan.wang@intel.com>.
> 

Sorry Laszlo, I didn't check the "Author" field before pushing the patch as I received the commit request form Fan. I don't know whether it's the patch reason or TortoiseGit failed to fetch the "Signed-off-by" tag as "Author" field. Anyway, I will double check that. 

> (2) The commit lists Siyuan and Jiaxin as having given their reviews. I
> don't see their *v2* reviews anywhere on the list. I see their *v1*
> reviews, but the patch was changed in v2. Wang Fan was correct not to
> carry forward the v1 reviews into the v2 posting. If the v2 posting was
> fine, then Siyuan and Jiaxin should have confirmed that (with R-b's) on
> the list, before pushing the commit.
> 

Yes, you are right, Fan should include the v1 reviewer into the v2 posting, and the reviewer should confirm the new version patch even he/she is the final version committer.

> Another process failure that I'm witnessing is that people forget to
> close BZs once the corresponding fixes are upstream. Sorry but that just
> makes a joke out of bug tracking.
> 

Thanks the reminder, we will pay attention next time.

> Come on, guys. We can do better than this. Show some discipline and
> dedication to the process, please. The process is not self-serving; it's
> in place to promote mutual understanding.
> 
> Laszlo
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel