public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v2 1/3] MdeModulePkg/Dhcp4Dxe: Remove unnecessary NULL pointer check.
@ 2019-01-18  5:16 Jiaxin Wu
  2019-01-18  5:22 ` Wu, Hao A
  0 siblings, 1 reply; 12+ messages in thread
From: Jiaxin Wu @ 2019-01-18  5:16 UTC (permalink / raw)
  To: edk2-devel; +Cc: Ye Ting, Fu Siyuan, Wu Hao A, Gao Liming, Wu Jiaxin

v2: The DHCP Instance might be destroyed in PxeDhcpDone. So,
we need safe-delete.

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1469

Since the value of Instance is retrieved from the list Entry,
it can't be the NULL pointer, so just remove the unnecessary
check.

Cc: Ye Ting <ting.ye@intel.com>
Cc: Fu Siyuan <siyuan.fu@intel.com>
Cc: Wu Hao A <hao.a.wu@intel.com>
Cc: Gao Liming <liming.gao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
---
 MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Io.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Io.c b/MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Io.c
index 98a22a77b4..780f8b4224 100644
--- a/MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Io.c
+++ b/MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Io.c
@@ -1,9 +1,9 @@
 /** @file
   EFI DHCP protocol implementation.
 
-Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD License
 which accompanies this distribution.  The full text of the license may be found at
 http://opensource.org/licenses/bsd-license.php
 
@@ -1646,16 +1646,13 @@ ON_EXIT:
   //
   // Iterate through all the DhcpSb Children.
   //
   NET_LIST_FOR_EACH_SAFE (Entry, Next, &DhcpSb->Children) {
     Instance = NET_LIST_USER_STRUCT (Entry, DHCP_PROTOCOL, Link);
-
-    if ((Instance != NULL) && (Instance->Token != NULL)) {
-      Instance->Timeout--;
-      if (Instance->Timeout == 0) {
-        PxeDhcpDone (Instance);
-      }
+    Instance->Timeout--;
+    if (Instance->Timeout == 0) {
+      PxeDhcpDone (Instance);
     }
   }
 
   return ;
 
-- 
2.17.1.windows.2



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 1/3] MdeModulePkg/Dhcp4Dxe: Remove unnecessary NULL pointer check.
  2019-01-18  5:16 [PATCH v2 1/3] MdeModulePkg/Dhcp4Dxe: Remove unnecessary NULL pointer check Jiaxin Wu
@ 2019-01-18  5:22 ` Wu, Hao A
  2019-01-18  5:29   ` Fu, Siyuan
  0 siblings, 1 reply; 12+ messages in thread
From: Wu, Hao A @ 2019-01-18  5:22 UTC (permalink / raw)
  To: Wu, Jiaxin, edk2-devel@lists.01.org; +Cc: Ye, Ting, Fu, Siyuan, Gao,  Liming

Hi Jiaxin,

A comment that is not related with the content of the patch itself:
Please help to send the full patch series when a new version is needed.

Best Regards,
Hao Wu

> -----Original Message-----
> From: Wu, Jiaxin
> Sent: Friday, January 18, 2019 1:16 PM
> To: edk2-devel@lists.01.org
> Cc: Ye, Ting; Fu, Siyuan; Wu, Hao A; Gao, Liming; Wu, Jiaxin
> Subject: [PATCH v2 1/3] MdeModulePkg/Dhcp4Dxe: Remove unnecessary
> NULL pointer check.
> 
> v2: The DHCP Instance might be destroyed in PxeDhcpDone. So,
> we need safe-delete.
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1469
> 
> Since the value of Instance is retrieved from the list Entry,
> it can't be the NULL pointer, so just remove the unnecessary
> check.
> 
> Cc: Ye Ting <ting.ye@intel.com>
> Cc: Fu Siyuan <siyuan.fu@intel.com>
> Cc: Wu Hao A <hao.a.wu@intel.com>
> Cc: Gao Liming <liming.gao@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
> ---
>  MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Io.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Io.c
> b/MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Io.c
> index 98a22a77b4..780f8b4224 100644
> --- a/MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Io.c
> +++ b/MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Io.c
> @@ -1,9 +1,9 @@
>  /** @file
>    EFI DHCP protocol implementation.
> 
> -Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
>  This program and the accompanying materials
>  are licensed and made available under the terms and conditions of the BSD
> License
>  which accompanies this distribution.  The full text of the license may be
> found at
>  http://opensource.org/licenses/bsd-license.php
> 
> @@ -1646,16 +1646,13 @@ ON_EXIT:
>    //
>    // Iterate through all the DhcpSb Children.
>    //
>    NET_LIST_FOR_EACH_SAFE (Entry, Next, &DhcpSb->Children) {
>      Instance = NET_LIST_USER_STRUCT (Entry, DHCP_PROTOCOL, Link);
> -
> -    if ((Instance != NULL) && (Instance->Token != NULL)) {
> -      Instance->Timeout--;
> -      if (Instance->Timeout == 0) {
> -        PxeDhcpDone (Instance);
> -      }
> +    Instance->Timeout--;
> +    if (Instance->Timeout == 0) {
> +      PxeDhcpDone (Instance);
>      }
>    }
> 
>    return ;
> 
> --
> 2.17.1.windows.2



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 1/3] MdeModulePkg/Dhcp4Dxe: Remove unnecessary NULL pointer check.
  2019-01-18  5:22 ` Wu, Hao A
@ 2019-01-18  5:29   ` Fu, Siyuan
  2019-01-18  5:31     ` Wu, Jiaxin
  0 siblings, 1 reply; 12+ messages in thread
From: Fu, Siyuan @ 2019-01-18  5:29 UTC (permalink / raw)
  To: Wu, Hao A, Wu, Jiaxin, edk2-devel@lists.01.org; +Cc: Ye, Ting, Gao, Liming

Hi, Jiaxin

Yes the full patch series is needed for a v2 version.

And also, why you removed the "(Instance->Token != NULL)" check in the if condition?

BestRegards
Fu Siyuan


> -----Original Message-----
> From: Wu, Hao A
> Sent: Friday, January 18, 2019 1:22 PM
> To: Wu, Jiaxin <jiaxin.wu@intel.com>; edk2-devel@lists.01.org
> Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Gao,
> Liming <liming.gao@intel.com>
> Subject: RE: [PATCH v2 1/3] MdeModulePkg/Dhcp4Dxe: Remove unnecessary NULL
> pointer check.
> 
> Hi Jiaxin,
> 
> A comment that is not related with the content of the patch itself:
> Please help to send the full patch series when a new version is needed.
> 
> Best Regards,
> Hao Wu
> 
> > -----Original Message-----
> > From: Wu, Jiaxin
> > Sent: Friday, January 18, 2019 1:16 PM
> > To: edk2-devel@lists.01.org
> > Cc: Ye, Ting; Fu, Siyuan; Wu, Hao A; Gao, Liming; Wu, Jiaxin
> > Subject: [PATCH v2 1/3] MdeModulePkg/Dhcp4Dxe: Remove unnecessary
> > NULL pointer check.
> >
> > v2: The DHCP Instance might be destroyed in PxeDhcpDone. So,
> > we need safe-delete.
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1469
> >
> > Since the value of Instance is retrieved from the list Entry,
> > it can't be the NULL pointer, so just remove the unnecessary
> > check.
> >
> > Cc: Ye Ting <ting.ye@intel.com>
> > Cc: Fu Siyuan <siyuan.fu@intel.com>
> > Cc: Wu Hao A <hao.a.wu@intel.com>
> > Cc: Gao Liming <liming.gao@intel.com>
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
> > ---
> >  MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Io.c | 11 ++++-------
> >  1 file changed, 4 insertions(+), 7 deletions(-)
> >
> > diff --git a/MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Io.c
> > b/MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Io.c
> > index 98a22a77b4..780f8b4224 100644
> > --- a/MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Io.c
> > +++ b/MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Io.c
> > @@ -1,9 +1,9 @@
> >  /** @file
> >    EFI DHCP protocol implementation.
> >
> > -Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
> > +Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
> >  This program and the accompanying materials
> >  are licensed and made available under the terms and conditions of the BSD
> > License
> >  which accompanies this distribution.  The full text of the license may be
> > found at
> >  http://opensource.org/licenses/bsd-license.php
> >
> > @@ -1646,16 +1646,13 @@ ON_EXIT:
> >    //
> >    // Iterate through all the DhcpSb Children.
> >    //
> >    NET_LIST_FOR_EACH_SAFE (Entry, Next, &DhcpSb->Children) {
> >      Instance = NET_LIST_USER_STRUCT (Entry, DHCP_PROTOCOL, Link);
> > -
> > -    if ((Instance != NULL) && (Instance->Token != NULL)) {
> > -      Instance->Timeout--;
> > -      if (Instance->Timeout == 0) {
> > -        PxeDhcpDone (Instance);
> > -      }
> > +    Instance->Timeout--;
> > +    if (Instance->Timeout == 0) {
> > +      PxeDhcpDone (Instance);
> >      }
> >    }
> >
> >    return ;
> >
> > --
> > 2.17.1.windows.2



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 1/3] MdeModulePkg/Dhcp4Dxe: Remove unnecessary NULL pointer check.
  2019-01-18  5:29   ` Fu, Siyuan
@ 2019-01-18  5:31     ` Wu, Jiaxin
  2019-01-18  5:38       ` Gao, Liming
  0 siblings, 1 reply; 12+ messages in thread
From: Wu, Jiaxin @ 2019-01-18  5:31 UTC (permalink / raw)
  To: Fu, Siyuan, Wu, Hao A, edk2-devel@lists.01.org; +Cc: Ye, Ting, Gao, Liming

Just confirmed with Liming, we don't need to seed the full series patches if only one is updated.

Thanks,
jiaxin

> -----Original Message-----
> From: Fu, Siyuan
> Sent: Friday, January 18, 2019 1:29 PM
> To: Wu, Hao A <hao.a.wu@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>;
> edk2-devel@lists.01.org
> Cc: Ye, Ting <ting.ye@intel.com>; Gao, Liming <liming.gao@intel.com>
> Subject: RE: [PATCH v2 1/3] MdeModulePkg/Dhcp4Dxe: Remove
> unnecessary NULL pointer check.
> 
> Hi, Jiaxin
> 
> Yes the full patch series is needed for a v2 version.
> 
> And also, why you removed the "(Instance->Token != NULL)" check in the if
> condition?
> 
> BestRegards
> Fu Siyuan
> 
> 
> > -----Original Message-----
> > From: Wu, Hao A
> > Sent: Friday, January 18, 2019 1:22 PM
> > To: Wu, Jiaxin <jiaxin.wu@intel.com>; edk2-devel@lists.01.org
> > Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Gao,
> > Liming <liming.gao@intel.com>
> > Subject: RE: [PATCH v2 1/3] MdeModulePkg/Dhcp4Dxe: Remove
> unnecessary NULL
> > pointer check.
> >
> > Hi Jiaxin,
> >
> > A comment that is not related with the content of the patch itself:
> > Please help to send the full patch series when a new version is needed.
> >
> > Best Regards,
> > Hao Wu
> >
> > > -----Original Message-----
> > > From: Wu, Jiaxin
> > > Sent: Friday, January 18, 2019 1:16 PM
> > > To: edk2-devel@lists.01.org
> > > Cc: Ye, Ting; Fu, Siyuan; Wu, Hao A; Gao, Liming; Wu, Jiaxin
> > > Subject: [PATCH v2 1/3] MdeModulePkg/Dhcp4Dxe: Remove
> unnecessary
> > > NULL pointer check.
> > >
> > > v2: The DHCP Instance might be destroyed in PxeDhcpDone. So,
> > > we need safe-delete.
> > >
> > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1469
> > >
> > > Since the value of Instance is retrieved from the list Entry,
> > > it can't be the NULL pointer, so just remove the unnecessary
> > > check.
> > >
> > > Cc: Ye Ting <ting.ye@intel.com>
> > > Cc: Fu Siyuan <siyuan.fu@intel.com>
> > > Cc: Wu Hao A <hao.a.wu@intel.com>
> > > Cc: Gao Liming <liming.gao@intel.com>
> > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
> > > ---
> > >  MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Io.c | 11 ++++-----
> --
> > >  1 file changed, 4 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Io.c
> > > b/MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Io.c
> > > index 98a22a77b4..780f8b4224 100644
> > > --- a/MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Io.c
> > > +++ b/MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Io.c
> > > @@ -1,9 +1,9 @@
> > >  /** @file
> > >    EFI DHCP protocol implementation.
> > >
> > > -Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
> > > +Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
> > >  This program and the accompanying materials
> > >  are licensed and made available under the terms and conditions of the
> BSD
> > > License
> > >  which accompanies this distribution.  The full text of the license may be
> > > found at
> > >  http://opensource.org/licenses/bsd-license.php
> > >
> > > @@ -1646,16 +1646,13 @@ ON_EXIT:
> > >    //
> > >    // Iterate through all the DhcpSb Children.
> > >    //
> > >    NET_LIST_FOR_EACH_SAFE (Entry, Next, &DhcpSb->Children) {
> > >      Instance = NET_LIST_USER_STRUCT (Entry, DHCP_PROTOCOL, Link);
> > > -
> > > -    if ((Instance != NULL) && (Instance->Token != NULL)) {
> > > -      Instance->Timeout--;
> > > -      if (Instance->Timeout == 0) {
> > > -        PxeDhcpDone (Instance);
> > > -      }
> > > +    Instance->Timeout--;
> > > +    if (Instance->Timeout == 0) {
> > > +      PxeDhcpDone (Instance);
> > >      }
> > >    }
> > >
> > >    return ;
> > >
> > > --
> > > 2.17.1.windows.2



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 1/3] MdeModulePkg/Dhcp4Dxe: Remove unnecessary NULL pointer check.
  2019-01-18  5:31     ` Wu, Jiaxin
@ 2019-01-18  5:38       ` Gao, Liming
  2019-01-18 11:09         ` Ard Biesheuvel
  0 siblings, 1 reply; 12+ messages in thread
From: Gao, Liming @ 2019-01-18  5:38 UTC (permalink / raw)
  To: Wu, Jiaxin, Fu, Siyuan, Wu, Hao A, edk2-devel@lists.01.org,
	Laszlo Ersek (lersek@redhat.com), ard.biesheuvel@linaro.org
  Cc: Ye, Ting, Gao, Liming

This is my idea to avoid the duplicated mail. I also include Ard and Laszlo to collect the feedback on how to handle the partial update in the patchset. 

Thanks
Liming
> -----Original Message-----
> From: Wu, Jiaxin
> Sent: Friday, January 18, 2019 1:32 PM
> To: Fu, Siyuan <siyuan.fu@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; edk2-devel@lists.01.org
> Cc: Ye, Ting <ting.ye@intel.com>; Gao, Liming <liming.gao@intel.com>
> Subject: RE: [PATCH v2 1/3] MdeModulePkg/Dhcp4Dxe: Remove unnecessary NULL pointer check.
> 
> Just confirmed with Liming, we don't need to seed the full series patches if only one is updated.
> 
> Thanks,
> jiaxin
> 
> > -----Original Message-----
> > From: Fu, Siyuan
> > Sent: Friday, January 18, 2019 1:29 PM
> > To: Wu, Hao A <hao.a.wu@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>;
> > edk2-devel@lists.01.org
> > Cc: Ye, Ting <ting.ye@intel.com>; Gao, Liming <liming.gao@intel.com>
> > Subject: RE: [PATCH v2 1/3] MdeModulePkg/Dhcp4Dxe: Remove
> > unnecessary NULL pointer check.
> >
> > Hi, Jiaxin
> >
> > Yes the full patch series is needed for a v2 version.
> >
> > And also, why you removed the "(Instance->Token != NULL)" check in the if
> > condition?
> >
> > BestRegards
> > Fu Siyuan
> >
> >
> > > -----Original Message-----
> > > From: Wu, Hao A
> > > Sent: Friday, January 18, 2019 1:22 PM
> > > To: Wu, Jiaxin <jiaxin.wu@intel.com>; edk2-devel@lists.01.org
> > > Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Gao,
> > > Liming <liming.gao@intel.com>
> > > Subject: RE: [PATCH v2 1/3] MdeModulePkg/Dhcp4Dxe: Remove
> > unnecessary NULL
> > > pointer check.
> > >
> > > Hi Jiaxin,
> > >
> > > A comment that is not related with the content of the patch itself:
> > > Please help to send the full patch series when a new version is needed.
> > >
> > > Best Regards,
> > > Hao Wu
> > >
> > > > -----Original Message-----
> > > > From: Wu, Jiaxin
> > > > Sent: Friday, January 18, 2019 1:16 PM
> > > > To: edk2-devel@lists.01.org
> > > > Cc: Ye, Ting; Fu, Siyuan; Wu, Hao A; Gao, Liming; Wu, Jiaxin
> > > > Subject: [PATCH v2 1/3] MdeModulePkg/Dhcp4Dxe: Remove
> > unnecessary
> > > > NULL pointer check.
> > > >
> > > > v2: The DHCP Instance might be destroyed in PxeDhcpDone. So,
> > > > we need safe-delete.
> > > >
> > > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1469
> > > >
> > > > Since the value of Instance is retrieved from the list Entry,
> > > > it can't be the NULL pointer, so just remove the unnecessary
> > > > check.
> > > >
> > > > Cc: Ye Ting <ting.ye@intel.com>
> > > > Cc: Fu Siyuan <siyuan.fu@intel.com>
> > > > Cc: Wu Hao A <hao.a.wu@intel.com>
> > > > Cc: Gao Liming <liming.gao@intel.com>
> > > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > > Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
> > > > ---
> > > >  MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Io.c | 11 ++++-----
> > --
> > > >  1 file changed, 4 insertions(+), 7 deletions(-)
> > > >
> > > > diff --git a/MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Io.c
> > > > b/MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Io.c
> > > > index 98a22a77b4..780f8b4224 100644
> > > > --- a/MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Io.c
> > > > +++ b/MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Io.c
> > > > @@ -1,9 +1,9 @@
> > > >  /** @file
> > > >    EFI DHCP protocol implementation.
> > > >
> > > > -Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
> > > > +Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
> > > >  This program and the accompanying materials
> > > >  are licensed and made available under the terms and conditions of the
> > BSD
> > > > License
> > > >  which accompanies this distribution.  The full text of the license may be
> > > > found at
> > > >  http://opensource.org/licenses/bsd-license.php
> > > >
> > > > @@ -1646,16 +1646,13 @@ ON_EXIT:
> > > >    //
> > > >    // Iterate through all the DhcpSb Children.
> > > >    //
> > > >    NET_LIST_FOR_EACH_SAFE (Entry, Next, &DhcpSb->Children) {
> > > >      Instance = NET_LIST_USER_STRUCT (Entry, DHCP_PROTOCOL, Link);
> > > > -
> > > > -    if ((Instance != NULL) && (Instance->Token != NULL)) {
> > > > -      Instance->Timeout--;
> > > > -      if (Instance->Timeout == 0) {
> > > > -        PxeDhcpDone (Instance);
> > > > -      }
> > > > +    Instance->Timeout--;
> > > > +    if (Instance->Timeout == 0) {
> > > > +      PxeDhcpDone (Instance);
> > > >      }
> > > >    }
> > > >
> > > >    return ;
> > > >
> > > > --
> > > > 2.17.1.windows.2



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 1/3] MdeModulePkg/Dhcp4Dxe: Remove unnecessary NULL pointer check.
  2019-01-18  5:38       ` Gao, Liming
@ 2019-01-18 11:09         ` Ard Biesheuvel
  2019-01-19  1:17           ` Laszlo Ersek
  0 siblings, 1 reply; 12+ messages in thread
From: Ard Biesheuvel @ 2019-01-18 11:09 UTC (permalink / raw)
  To: Gao, Liming
  Cc: Wu, Jiaxin, Fu, Siyuan, Wu, Hao A, edk2-devel@lists.01.org,
	Laszlo Ersek (lersek@redhat.com), Ye, Ting

On Fri, 18 Jan 2019 at 06:38, Gao, Liming <liming.gao@intel.com> wrote:
>
> This is my idea to avoid the duplicated mail. I also include Ard and Laszlo to collect the feedback on how to handle the partial update in the patchset.
>

Laszlo may disagree with me, but I think that it is not always
necessary to resend the entire series when only a single patch
changes. It does depend on the situation, though: if it is a trivial
patch in a more complicated series then it might make little sense. In
other case, just resending the whole thing is probably better.


> > -----Original Message-----
> > From: Wu, Jiaxin
> > Sent: Friday, January 18, 2019 1:32 PM
> > To: Fu, Siyuan <siyuan.fu@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; edk2-devel@lists.01.org
> > Cc: Ye, Ting <ting.ye@intel.com>; Gao, Liming <liming.gao@intel.com>
> > Subject: RE: [PATCH v2 1/3] MdeModulePkg/Dhcp4Dxe: Remove unnecessary NULL pointer check.
> >
> > Just confirmed with Liming, we don't need to seed the full series patches if only one is updated.
> >
> > Thanks,
> > jiaxin
> >
> > > -----Original Message-----
> > > From: Fu, Siyuan
> > > Sent: Friday, January 18, 2019 1:29 PM
> > > To: Wu, Hao A <hao.a.wu@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>;
> > > edk2-devel@lists.01.org
> > > Cc: Ye, Ting <ting.ye@intel.com>; Gao, Liming <liming.gao@intel.com>
> > > Subject: RE: [PATCH v2 1/3] MdeModulePkg/Dhcp4Dxe: Remove
> > > unnecessary NULL pointer check.
> > >
> > > Hi, Jiaxin
> > >
> > > Yes the full patch series is needed for a v2 version.
> > >
> > > And also, why you removed the "(Instance->Token != NULL)" check in the if
> > > condition?
> > >
> > > BestRegards
> > > Fu Siyuan
> > >
> > >
> > > > -----Original Message-----
> > > > From: Wu, Hao A
> > > > Sent: Friday, January 18, 2019 1:22 PM
> > > > To: Wu, Jiaxin <jiaxin.wu@intel.com>; edk2-devel@lists.01.org
> > > > Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Gao,
> > > > Liming <liming.gao@intel.com>
> > > > Subject: RE: [PATCH v2 1/3] MdeModulePkg/Dhcp4Dxe: Remove
> > > unnecessary NULL
> > > > pointer check.
> > > >
> > > > Hi Jiaxin,
> > > >
> > > > A comment that is not related with the content of the patch itself:
> > > > Please help to send the full patch series when a new version is needed.
> > > >
> > > > Best Regards,
> > > > Hao Wu
> > > >
> > > > > -----Original Message-----
> > > > > From: Wu, Jiaxin
> > > > > Sent: Friday, January 18, 2019 1:16 PM
> > > > > To: edk2-devel@lists.01.org
> > > > > Cc: Ye, Ting; Fu, Siyuan; Wu, Hao A; Gao, Liming; Wu, Jiaxin
> > > > > Subject: [PATCH v2 1/3] MdeModulePkg/Dhcp4Dxe: Remove
> > > unnecessary
> > > > > NULL pointer check.
> > > > >
> > > > > v2: The DHCP Instance might be destroyed in PxeDhcpDone. So,
> > > > > we need safe-delete.
> > > > >
> > > > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1469
> > > > >
> > > > > Since the value of Instance is retrieved from the list Entry,
> > > > > it can't be the NULL pointer, so just remove the unnecessary
> > > > > check.
> > > > >
> > > > > Cc: Ye Ting <ting.ye@intel.com>
> > > > > Cc: Fu Siyuan <siyuan.fu@intel.com>
> > > > > Cc: Wu Hao A <hao.a.wu@intel.com>
> > > > > Cc: Gao Liming <liming.gao@intel.com>
> > > > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > > > Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
> > > > > ---
> > > > >  MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Io.c | 11 ++++-----
> > > --
> > > > >  1 file changed, 4 insertions(+), 7 deletions(-)
> > > > >
> > > > > diff --git a/MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Io.c
> > > > > b/MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Io.c
> > > > > index 98a22a77b4..780f8b4224 100644
> > > > > --- a/MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Io.c
> > > > > +++ b/MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Io.c
> > > > > @@ -1,9 +1,9 @@
> > > > >  /** @file
> > > > >    EFI DHCP protocol implementation.
> > > > >
> > > > > -Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
> > > > > +Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
> > > > >  This program and the accompanying materials
> > > > >  are licensed and made available under the terms and conditions of the
> > > BSD
> > > > > License
> > > > >  which accompanies this distribution.  The full text of the license may be
> > > > > found at
> > > > >  http://opensource.org/licenses/bsd-license.php
> > > > >
> > > > > @@ -1646,16 +1646,13 @@ ON_EXIT:
> > > > >    //
> > > > >    // Iterate through all the DhcpSb Children.
> > > > >    //
> > > > >    NET_LIST_FOR_EACH_SAFE (Entry, Next, &DhcpSb->Children) {
> > > > >      Instance = NET_LIST_USER_STRUCT (Entry, DHCP_PROTOCOL, Link);
> > > > > -
> > > > > -    if ((Instance != NULL) && (Instance->Token != NULL)) {
> > > > > -      Instance->Timeout--;
> > > > > -      if (Instance->Timeout == 0) {
> > > > > -        PxeDhcpDone (Instance);
> > > > > -      }
> > > > > +    Instance->Timeout--;
> > > > > +    if (Instance->Timeout == 0) {
> > > > > +      PxeDhcpDone (Instance);
> > > > >      }
> > > > >    }
> > > > >
> > > > >    return ;
> > > > >
> > > > > --
> > > > > 2.17.1.windows.2
>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 1/3] MdeModulePkg/Dhcp4Dxe: Remove unnecessary NULL pointer check.
  2019-01-18 11:09         ` Ard Biesheuvel
@ 2019-01-19  1:17           ` Laszlo Ersek
  2019-01-20 18:25             ` Wu, Jiaxin
  2019-01-21 12:53             ` Gao, Liming
  0 siblings, 2 replies; 12+ messages in thread
From: Laszlo Ersek @ 2019-01-19  1:17 UTC (permalink / raw)
  To: Ard Biesheuvel, Gao, Liming
  Cc: Wu, Jiaxin, Fu, Siyuan, Wu, Hao A, edk2-devel@lists.01.org,
	Ye, Ting

On 01/18/19 12:09, Ard Biesheuvel wrote:
> On Fri, 18 Jan 2019 at 06:38, Gao, Liming <liming.gao@intel.com> wrote:
>>
>> This is my idea to avoid the duplicated mail. I also include Ard and Laszlo to collect the feedback on how to handle the partial update in the patchset.
>>
> 
> Laszlo may disagree with me, but I think that it is not always
> necessary to resend the entire series when only a single patch
> changes. It does depend on the situation, though: if it is a trivial
> patch in a more complicated series then it might make little sense. In
> other case, just resending the whole thing is probably better.

I think resending one patch can be acceptable, but the subject line
(patch nr) and the threading have to be correct. Also, I don't think
this approach scales beyond exactly one patch-update; it's easy to lose
track of version numbers etc. So "use sparingly" I guess? :)

Thanks
Laszlo

> 
> 
>>> -----Original Message-----
>>> From: Wu, Jiaxin
>>> Sent: Friday, January 18, 2019 1:32 PM
>>> To: Fu, Siyuan <siyuan.fu@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; edk2-devel@lists.01.org
>>> Cc: Ye, Ting <ting.ye@intel.com>; Gao, Liming <liming.gao@intel.com>
>>> Subject: RE: [PATCH v2 1/3] MdeModulePkg/Dhcp4Dxe: Remove unnecessary NULL pointer check.
>>>
>>> Just confirmed with Liming, we don't need to seed the full series patches if only one is updated.
>>>
>>> Thanks,
>>> jiaxin
>>>
>>>> -----Original Message-----
>>>> From: Fu, Siyuan
>>>> Sent: Friday, January 18, 2019 1:29 PM
>>>> To: Wu, Hao A <hao.a.wu@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>;
>>>> edk2-devel@lists.01.org
>>>> Cc: Ye, Ting <ting.ye@intel.com>; Gao, Liming <liming.gao@intel.com>
>>>> Subject: RE: [PATCH v2 1/3] MdeModulePkg/Dhcp4Dxe: Remove
>>>> unnecessary NULL pointer check.
>>>>
>>>> Hi, Jiaxin
>>>>
>>>> Yes the full patch series is needed for a v2 version.
>>>>
>>>> And also, why you removed the "(Instance->Token != NULL)" check in the if
>>>> condition?
>>>>
>>>> BestRegards
>>>> Fu Siyuan
>>>>
>>>>
>>>>> -----Original Message-----
>>>>> From: Wu, Hao A
>>>>> Sent: Friday, January 18, 2019 1:22 PM
>>>>> To: Wu, Jiaxin <jiaxin.wu@intel.com>; edk2-devel@lists.01.org
>>>>> Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Gao,
>>>>> Liming <liming.gao@intel.com>
>>>>> Subject: RE: [PATCH v2 1/3] MdeModulePkg/Dhcp4Dxe: Remove
>>>> unnecessary NULL
>>>>> pointer check.
>>>>>
>>>>> Hi Jiaxin,
>>>>>
>>>>> A comment that is not related with the content of the patch itself:
>>>>> Please help to send the full patch series when a new version is needed.
>>>>>
>>>>> Best Regards,
>>>>> Hao Wu
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Wu, Jiaxin
>>>>>> Sent: Friday, January 18, 2019 1:16 PM
>>>>>> To: edk2-devel@lists.01.org
>>>>>> Cc: Ye, Ting; Fu, Siyuan; Wu, Hao A; Gao, Liming; Wu, Jiaxin
>>>>>> Subject: [PATCH v2 1/3] MdeModulePkg/Dhcp4Dxe: Remove
>>>> unnecessary
>>>>>> NULL pointer check.
>>>>>>
>>>>>> v2: The DHCP Instance might be destroyed in PxeDhcpDone. So,
>>>>>> we need safe-delete.
>>>>>>
>>>>>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1469
>>>>>>
>>>>>> Since the value of Instance is retrieved from the list Entry,
>>>>>> it can't be the NULL pointer, so just remove the unnecessary
>>>>>> check.
>>>>>>
>>>>>> Cc: Ye Ting <ting.ye@intel.com>
>>>>>> Cc: Fu Siyuan <siyuan.fu@intel.com>
>>>>>> Cc: Wu Hao A <hao.a.wu@intel.com>
>>>>>> Cc: Gao Liming <liming.gao@intel.com>
>>>>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>>>>> Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
>>>>>> ---
>>>>>>  MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Io.c | 11 ++++-----
>>>> --
>>>>>>  1 file changed, 4 insertions(+), 7 deletions(-)
>>>>>>
>>>>>> diff --git a/MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Io.c
>>>>>> b/MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Io.c
>>>>>> index 98a22a77b4..780f8b4224 100644
>>>>>> --- a/MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Io.c
>>>>>> +++ b/MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Io.c
>>>>>> @@ -1,9 +1,9 @@
>>>>>>  /** @file
>>>>>>    EFI DHCP protocol implementation.
>>>>>>
>>>>>> -Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
>>>>>> +Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
>>>>>>  This program and the accompanying materials
>>>>>>  are licensed and made available under the terms and conditions of the
>>>> BSD
>>>>>> License
>>>>>>  which accompanies this distribution.  The full text of the license may be
>>>>>> found at
>>>>>>  http://opensource.org/licenses/bsd-license.php
>>>>>>
>>>>>> @@ -1646,16 +1646,13 @@ ON_EXIT:
>>>>>>    //
>>>>>>    // Iterate through all the DhcpSb Children.
>>>>>>    //
>>>>>>    NET_LIST_FOR_EACH_SAFE (Entry, Next, &DhcpSb->Children) {
>>>>>>      Instance = NET_LIST_USER_STRUCT (Entry, DHCP_PROTOCOL, Link);
>>>>>> -
>>>>>> -    if ((Instance != NULL) && (Instance->Token != NULL)) {
>>>>>> -      Instance->Timeout--;
>>>>>> -      if (Instance->Timeout == 0) {
>>>>>> -        PxeDhcpDone (Instance);
>>>>>> -      }
>>>>>> +    Instance->Timeout--;
>>>>>> +    if (Instance->Timeout == 0) {
>>>>>> +      PxeDhcpDone (Instance);
>>>>>>      }
>>>>>>    }
>>>>>>
>>>>>>    return ;
>>>>>>
>>>>>> --
>>>>>> 2.17.1.windows.2
>>



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 1/3] MdeModulePkg/Dhcp4Dxe: Remove unnecessary NULL pointer check.
  2019-01-19  1:17           ` Laszlo Ersek
@ 2019-01-20 18:25             ` Wu, Jiaxin
  2019-01-21 12:53             ` Gao, Liming
  1 sibling, 0 replies; 12+ messages in thread
From: Wu, Jiaxin @ 2019-01-20 18:25 UTC (permalink / raw)
  To: Laszlo Ersek, Ard Biesheuvel, Gao, Liming
  Cc: Fu, Siyuan, Wu, Hao A, edk2-devel@lists.01.org, Ye, Ting

> >> This is my idea to avoid the duplicated mail. I also include Ard and Laszlo to
> collect the feedback on how to handle the partial update in the patchset.
> >>
> >
> > Laszlo may disagree with me, but I think that it is not always
> > necessary to resend the entire series when only a single patch
> > changes. It does depend on the situation, though: if it is a trivial
> > patch in a more complicated series then it might make little sense. In
> > other case, just resending the whole thing is probably better.
> 
> I think resending one patch can be acceptable, but the subject line
> (patch nr) and the threading have to be correct. Also, I don't think
> this approach scales beyond exactly one patch-update; it's easy to lose
> track of version numbers etc. So "use sparingly" I guess? :)
> 

Thanks all of your comments, to avoid the missing version track, I have resent the whole patch to version 3:).

Best Regard!
Jiaxin



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 1/3] MdeModulePkg/Dhcp4Dxe: Remove unnecessary NULL pointer check.
  2019-01-19  1:17           ` Laszlo Ersek
  2019-01-20 18:25             ` Wu, Jiaxin
@ 2019-01-21 12:53             ` Gao, Liming
  2019-01-21 13:26               ` Philippe Mathieu-Daudé
  1 sibling, 1 reply; 12+ messages in thread
From: Gao, Liming @ 2019-01-21 12:53 UTC (permalink / raw)
  To: Laszlo Ersek, Ard Biesheuvel
  Cc: Wu, Jiaxin, Fu, Siyuan, Wu, Hao A, edk2-devel@lists.01.org,
	Ye, Ting

Thanks Ard and Laszlo. For the minor change in single patch, the patch may be sent separately with the clear subject. Or, the patch set can be sent again.

Thanks
Liming
> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Saturday, January 19, 2019 9:17 AM
> To: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Gao, Liming <liming.gao@intel.com>
> Cc: Wu, Jiaxin <jiaxin.wu@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; edk2-devel@lists.01.org; Ye,
> Ting <ting.ye@intel.com>
> Subject: Re: [PATCH v2 1/3] MdeModulePkg/Dhcp4Dxe: Remove unnecessary NULL pointer check.
> 
> On 01/18/19 12:09, Ard Biesheuvel wrote:
> > On Fri, 18 Jan 2019 at 06:38, Gao, Liming <liming.gao@intel.com> wrote:
> >>
> >> This is my idea to avoid the duplicated mail. I also include Ard and Laszlo to collect the feedback on how to handle the partial update in
> the patchset.
> >>
> >
> > Laszlo may disagree with me, but I think that it is not always
> > necessary to resend the entire series when only a single patch
> > changes. It does depend on the situation, though: if it is a trivial
> > patch in a more complicated series then it might make little sense. In
> > other case, just resending the whole thing is probably better.
> 
> I think resending one patch can be acceptable, but the subject line
> (patch nr) and the threading have to be correct. Also, I don't think
> this approach scales beyond exactly one patch-update; it's easy to lose
> track of version numbers etc. So "use sparingly" I guess? :)
> 
> Thanks
> Laszlo
> 
> >
> >
> >>> -----Original Message-----
> >>> From: Wu, Jiaxin
> >>> Sent: Friday, January 18, 2019 1:32 PM
> >>> To: Fu, Siyuan <siyuan.fu@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; edk2-devel@lists.01.org
> >>> Cc: Ye, Ting <ting.ye@intel.com>; Gao, Liming <liming.gao@intel.com>
> >>> Subject: RE: [PATCH v2 1/3] MdeModulePkg/Dhcp4Dxe: Remove unnecessary NULL pointer check.
> >>>
> >>> Just confirmed with Liming, we don't need to seed the full series patches if only one is updated.
> >>>
> >>> Thanks,
> >>> jiaxin
> >>>
> >>>> -----Original Message-----
> >>>> From: Fu, Siyuan
> >>>> Sent: Friday, January 18, 2019 1:29 PM
> >>>> To: Wu, Hao A <hao.a.wu@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>;
> >>>> edk2-devel@lists.01.org
> >>>> Cc: Ye, Ting <ting.ye@intel.com>; Gao, Liming <liming.gao@intel.com>
> >>>> Subject: RE: [PATCH v2 1/3] MdeModulePkg/Dhcp4Dxe: Remove
> >>>> unnecessary NULL pointer check.
> >>>>
> >>>> Hi, Jiaxin
> >>>>
> >>>> Yes the full patch series is needed for a v2 version.
> >>>>
> >>>> And also, why you removed the "(Instance->Token != NULL)" check in the if
> >>>> condition?
> >>>>
> >>>> BestRegards
> >>>> Fu Siyuan
> >>>>
> >>>>
> >>>>> -----Original Message-----
> >>>>> From: Wu, Hao A
> >>>>> Sent: Friday, January 18, 2019 1:22 PM
> >>>>> To: Wu, Jiaxin <jiaxin.wu@intel.com>; edk2-devel@lists.01.org
> >>>>> Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Gao,
> >>>>> Liming <liming.gao@intel.com>
> >>>>> Subject: RE: [PATCH v2 1/3] MdeModulePkg/Dhcp4Dxe: Remove
> >>>> unnecessary NULL
> >>>>> pointer check.
> >>>>>
> >>>>> Hi Jiaxin,
> >>>>>
> >>>>> A comment that is not related with the content of the patch itself:
> >>>>> Please help to send the full patch series when a new version is needed.
> >>>>>
> >>>>> Best Regards,
> >>>>> Hao Wu
> >>>>>
> >>>>>> -----Original Message-----
> >>>>>> From: Wu, Jiaxin
> >>>>>> Sent: Friday, January 18, 2019 1:16 PM
> >>>>>> To: edk2-devel@lists.01.org
> >>>>>> Cc: Ye, Ting; Fu, Siyuan; Wu, Hao A; Gao, Liming; Wu, Jiaxin
> >>>>>> Subject: [PATCH v2 1/3] MdeModulePkg/Dhcp4Dxe: Remove
> >>>> unnecessary
> >>>>>> NULL pointer check.
> >>>>>>
> >>>>>> v2: The DHCP Instance might be destroyed in PxeDhcpDone. So,
> >>>>>> we need safe-delete.
> >>>>>>
> >>>>>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1469
> >>>>>>
> >>>>>> Since the value of Instance is retrieved from the list Entry,
> >>>>>> it can't be the NULL pointer, so just remove the unnecessary
> >>>>>> check.
> >>>>>>
> >>>>>> Cc: Ye Ting <ting.ye@intel.com>
> >>>>>> Cc: Fu Siyuan <siyuan.fu@intel.com>
> >>>>>> Cc: Wu Hao A <hao.a.wu@intel.com>
> >>>>>> Cc: Gao Liming <liming.gao@intel.com>
> >>>>>> Contributed-under: TianoCore Contribution Agreement 1.1
> >>>>>> Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
> >>>>>> ---
> >>>>>>  MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Io.c | 11 ++++-----
> >>>> --
> >>>>>>  1 file changed, 4 insertions(+), 7 deletions(-)
> >>>>>>
> >>>>>> diff --git a/MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Io.c
> >>>>>> b/MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Io.c
> >>>>>> index 98a22a77b4..780f8b4224 100644
> >>>>>> --- a/MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Io.c
> >>>>>> +++ b/MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Io.c
> >>>>>> @@ -1,9 +1,9 @@
> >>>>>>  /** @file
> >>>>>>    EFI DHCP protocol implementation.
> >>>>>>
> >>>>>> -Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
> >>>>>> +Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
> >>>>>>  This program and the accompanying materials
> >>>>>>  are licensed and made available under the terms and conditions of the
> >>>> BSD
> >>>>>> License
> >>>>>>  which accompanies this distribution.  The full text of the license may be
> >>>>>> found at
> >>>>>>  http://opensource.org/licenses/bsd-license.php
> >>>>>>
> >>>>>> @@ -1646,16 +1646,13 @@ ON_EXIT:
> >>>>>>    //
> >>>>>>    // Iterate through all the DhcpSb Children.
> >>>>>>    //
> >>>>>>    NET_LIST_FOR_EACH_SAFE (Entry, Next, &DhcpSb->Children) {
> >>>>>>      Instance = NET_LIST_USER_STRUCT (Entry, DHCP_PROTOCOL, Link);
> >>>>>> -
> >>>>>> -    if ((Instance != NULL) && (Instance->Token != NULL)) {
> >>>>>> -      Instance->Timeout--;
> >>>>>> -      if (Instance->Timeout == 0) {
> >>>>>> -        PxeDhcpDone (Instance);
> >>>>>> -      }
> >>>>>> +    Instance->Timeout--;
> >>>>>> +    if (Instance->Timeout == 0) {
> >>>>>> +      PxeDhcpDone (Instance);
> >>>>>>      }
> >>>>>>    }
> >>>>>>
> >>>>>>    return ;
> >>>>>>
> >>>>>> --
> >>>>>> 2.17.1.windows.2
> >>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 1/3] MdeModulePkg/Dhcp4Dxe: Remove unnecessary NULL pointer check.
  2019-01-21 12:53             ` Gao, Liming
@ 2019-01-21 13:26               ` Philippe Mathieu-Daudé
  2019-01-21 21:21                 ` Laszlo Ersek
  0 siblings, 1 reply; 12+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-01-21 13:26 UTC (permalink / raw)
  To: Gao, Liming, Laszlo Ersek, Ard Biesheuvel
  Cc: Wu, Hao A, Ye, Ting, Fu, Siyuan, Wu, Jiaxin,
	edk2-devel@lists.01.org

Hi,

On 1/21/19 1:53 PM, Gao, Liming wrote:
> Thanks Ard and Laszlo. For the minor change in single patch, the patch may be sent separately with the clear subject. Or, the patch set can be sent again.

Since it is hard to follow technical discussion when top-posted (see
https://www.caliburn.nl/topposting.html) without scrolling and sometime
loosing context, can we gently suggest bottom-posting in edk2-devel
etiquette? (No offence, this is a humble suggestion from a not very
active reviewer to a highly active contributor, but this might ease the
on-list review workflow).

>> -----Original Message-----
>> From: Laszlo Ersek [mailto:lersek@redhat.com]
>> Sent: Saturday, January 19, 2019 9:17 AM
>> To: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Gao, Liming <liming.gao@intel.com>
>> Cc: Wu, Jiaxin <jiaxin.wu@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; edk2-devel@lists.01.org; Ye,
>> Ting <ting.ye@intel.com>
>> Subject: Re: [PATCH v2 1/3] MdeModulePkg/Dhcp4Dxe: Remove unnecessary NULL pointer check.
>>
>> On 01/18/19 12:09, Ard Biesheuvel wrote:
>>> On Fri, 18 Jan 2019 at 06:38, Gao, Liming <liming.gao@intel.com> wrote:
>>>>
>>>> This is my idea to avoid the duplicated mail. I also include Ard and Laszlo to collect the feedback on how to handle the partial update in
>> the patchset.
>>>>
>>>
>>> Laszlo may disagree with me, but I think that it is not always
>>> necessary to resend the entire series when only a single patch
>>> changes. It does depend on the situation, though: if it is a trivial
>>> patch in a more complicated series then it might make little sense. In
>>> other case, just resending the whole thing is probably better.
>>
>> I think resending one patch can be acceptable, but the subject line
>> (patch nr) and the threading have to be correct. Also, I don't think
>> this approach scales beyond exactly one patch-update; it's easy to lose
>> track of version numbers etc. So "use sparingly" I guess? :)

For a 3 patches series, I wouldn't worry resending the whole series...

The 'git backport-diff' tool is very powerful to resume differencies
between 2 series, in particular when the project evolved between
versions of a series (simplest example: a rebase):

https://github.com/codyprime/git-scripts/blob/master/git-backport-diff#L27

Sadly I can't find a distribution handy package that provides it, so it
has to be installed manually.

Regards,

Phil.

>>>>> -----Original Message-----
>>>>> From: Wu, Jiaxin
>>>>> Sent: Friday, January 18, 2019 1:32 PM
>>>>> To: Fu, Siyuan <siyuan.fu@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; edk2-devel@lists.01.org
>>>>> Cc: Ye, Ting <ting.ye@intel.com>; Gao, Liming <liming.gao@intel.com>
>>>>> Subject: RE: [PATCH v2 1/3] MdeModulePkg/Dhcp4Dxe: Remove unnecessary NULL pointer check.
>>>>>
>>>>> Just confirmed with Liming, we don't need to seed the full series patches if only one is updated.
>>>>>
>>>>> Thanks,
>>>>> jiaxin
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Fu, Siyuan
>>>>>> Sent: Friday, January 18, 2019 1:29 PM
>>>>>> To: Wu, Hao A <hao.a.wu@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>;
>>>>>> edk2-devel@lists.01.org
>>>>>> Cc: Ye, Ting <ting.ye@intel.com>; Gao, Liming <liming.gao@intel.com>
>>>>>> Subject: RE: [PATCH v2 1/3] MdeModulePkg/Dhcp4Dxe: Remove
>>>>>> unnecessary NULL pointer check.
>>>>>>
>>>>>> Hi, Jiaxin
>>>>>>
>>>>>> Yes the full patch series is needed for a v2 version.
>>>>>>
>>>>>> And also, why you removed the "(Instance->Token != NULL)" check in the if
>>>>>> condition?
>>>>>>
>>>>>> BestRegards
>>>>>> Fu Siyuan
>>>>>>
>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: Wu, Hao A
>>>>>>> Sent: Friday, January 18, 2019 1:22 PM
>>>>>>> To: Wu, Jiaxin <jiaxin.wu@intel.com>; edk2-devel@lists.01.org
>>>>>>> Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Gao,
>>>>>>> Liming <liming.gao@intel.com>
>>>>>>> Subject: RE: [PATCH v2 1/3] MdeModulePkg/Dhcp4Dxe: Remove
>>>>>> unnecessary NULL
>>>>>>> pointer check.
>>>>>>>
>>>>>>> Hi Jiaxin,
>>>>>>>
>>>>>>> A comment that is not related with the content of the patch itself:
>>>>>>> Please help to send the full patch series when a new version is needed.
>>>>>>>
>>>>>>> Best Regards,
>>>>>>> Hao Wu
>>>>>>>
>>>>>>>> -----Original Message-----
>>>>>>>> From: Wu, Jiaxin
>>>>>>>> Sent: Friday, January 18, 2019 1:16 PM
>>>>>>>> To: edk2-devel@lists.01.org
>>>>>>>> Cc: Ye, Ting; Fu, Siyuan; Wu, Hao A; Gao, Liming; Wu, Jiaxin
>>>>>>>> Subject: [PATCH v2 1/3] MdeModulePkg/Dhcp4Dxe: Remove
>>>>>> unnecessary
>>>>>>>> NULL pointer check.
>>>>>>>>
>>>>>>>> v2: The DHCP Instance might be destroyed in PxeDhcpDone. So,
>>>>>>>> we need safe-delete.
>>>>>>>>
>>>>>>>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1469
>>>>>>>>
>>>>>>>> Since the value of Instance is retrieved from the list Entry,
>>>>>>>> it can't be the NULL pointer, so just remove the unnecessary
>>>>>>>> check.
>>>>>>>>
>>>>>>>> Cc: Ye Ting <ting.ye@intel.com>
>>>>>>>> Cc: Fu Siyuan <siyuan.fu@intel.com>
>>>>>>>> Cc: Wu Hao A <hao.a.wu@intel.com>
>>>>>>>> Cc: Gao Liming <liming.gao@intel.com>
>>>>>>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>>>>>>> Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
>>>>>>>> ---
>>>>>>>>  MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Io.c | 11 ++++-----
>>>>>> --
>>>>>>>>  1 file changed, 4 insertions(+), 7 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Io.c
>>>>>>>> b/MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Io.c
>>>>>>>> index 98a22a77b4..780f8b4224 100644
>>>>>>>> --- a/MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Io.c
>>>>>>>> +++ b/MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Io.c
>>>>>>>> @@ -1,9 +1,9 @@
>>>>>>>>  /** @file
>>>>>>>>    EFI DHCP protocol implementation.
>>>>>>>>
>>>>>>>> -Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
>>>>>>>> +Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
>>>>>>>>  This program and the accompanying materials
>>>>>>>>  are licensed and made available under the terms and conditions of the
>>>>>> BSD
>>>>>>>> License
>>>>>>>>  which accompanies this distribution.  The full text of the license may be
>>>>>>>> found at
>>>>>>>>  http://opensource.org/licenses/bsd-license.php
>>>>>>>>
>>>>>>>> @@ -1646,16 +1646,13 @@ ON_EXIT:
>>>>>>>>    //
>>>>>>>>    // Iterate through all the DhcpSb Children.
>>>>>>>>    //
>>>>>>>>    NET_LIST_FOR_EACH_SAFE (Entry, Next, &DhcpSb->Children) {
>>>>>>>>      Instance = NET_LIST_USER_STRUCT (Entry, DHCP_PROTOCOL, Link);
>>>>>>>> -
>>>>>>>> -    if ((Instance != NULL) && (Instance->Token != NULL)) {
>>>>>>>> -      Instance->Timeout--;
>>>>>>>> -      if (Instance->Timeout == 0) {
>>>>>>>> -        PxeDhcpDone (Instance);
>>>>>>>> -      }
>>>>>>>> +    Instance->Timeout--;
>>>>>>>> +    if (Instance->Timeout == 0) {
>>>>>>>> +      PxeDhcpDone (Instance);
>>>>>>>>      }
>>>>>>>>    }
>>>>>>>>
>>>>>>>>    return ;
>>>>>>>>
>>>>>>>> --
>>>>>>>> 2.17.1.windows.2


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 1/3] MdeModulePkg/Dhcp4Dxe: Remove unnecessary NULL pointer check.
  2019-01-21 13:26               ` Philippe Mathieu-Daudé
@ 2019-01-21 21:21                 ` Laszlo Ersek
  2019-01-21 22:47                   ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 12+ messages in thread
From: Laszlo Ersek @ 2019-01-21 21:21 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, Gao, Liming, Ard Biesheuvel
  Cc: Wu, Hao A, Ye, Ting, Fu, Siyuan, Wu, Jiaxin,
	edk2-devel@lists.01.org

Hi Phil,

On 01/21/19 14:26, Philippe Mathieu-Daudé wrote:
> Hi,
> 
> On 1/21/19 1:53 PM, Gao, Liming wrote:
>> Thanks Ard and Laszlo. For the minor change in single patch, the patch may be sent separately with the clear subject. Or, the patch set can be sent again.
> 
> Since it is hard to follow technical discussion when top-posted (see
> https://www.caliburn.nl/topposting.html) without scrolling and sometime
> loosing context, can we gently suggest bottom-posting in edk2-devel
> etiquette? (No offence, this is a humble suggestion from a not very
> active reviewer to a highly active contributor, but this might ease the
> on-list review workflow).

top vs. bottom posting have been mentioned multiple times on edk2-devel;
it's just a fact that most corporate email environments don't support
bottom posting at all.

I'm unhappy about it (obviously), but it's an uphill battle. Sometimes
the poster would actually *like* to bottom post, but the tooling (which
may not be their own choice) gets in their way.

I suggest always scrolling to the bottom, or at least until you see a
signature.

Thanks,
Laszlo


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 1/3] MdeModulePkg/Dhcp4Dxe: Remove unnecessary NULL pointer check.
  2019-01-21 21:21                 ` Laszlo Ersek
@ 2019-01-21 22:47                   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 12+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-01-21 22:47 UTC (permalink / raw)
  To: Laszlo Ersek, Gao, Liming, Ard Biesheuvel
  Cc: Wu, Hao A, Ye, Ting, Fu, Siyuan, Wu, Jiaxin,
	edk2-devel@lists.01.org

On 1/21/19 10:21 PM, Laszlo Ersek wrote:
> Hi Phil,
> 
> On 01/21/19 14:26, Philippe Mathieu-Daudé wrote:
>> Hi,
>>
>> On 1/21/19 1:53 PM, Gao, Liming wrote:
>>> Thanks Ard and Laszlo. For the minor change in single patch, the patch may be sent separately with the clear subject. Or, the patch set can be sent again.
>>
>> Since it is hard to follow technical discussion when top-posted (see
>> https://www.caliburn.nl/topposting.html) without scrolling and sometime
>> loosing context, can we gently suggest bottom-posting in edk2-devel
>> etiquette? (No offence, this is a humble suggestion from a not very
>> active reviewer to a highly active contributor, but this might ease the
>> on-list review workflow).
> 
> top vs. bottom posting have been mentioned multiple times on edk2-devel;
> it's just a fact that most corporate email environments don't support
> bottom posting at all.
> 
> I'm unhappy about it (obviously), but it's an uphill battle. Sometimes
> the poster would actually *like* to bottom post, but the tooling (which
> may not be their own choice) gets in their way.
> 
> I suggest always scrolling to the bottom, or at least until you see a
> signature.

Oh, I was not aware of that, it makes now sense (there is a common
pattern in companies/location with top-posts).
Sorry for the noise, at least I tried ;)

Regards,

Phil.

> 
> Thanks,
> Laszlo
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2019-01-21 22:47 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-01-18  5:16 [PATCH v2 1/3] MdeModulePkg/Dhcp4Dxe: Remove unnecessary NULL pointer check Jiaxin Wu
2019-01-18  5:22 ` Wu, Hao A
2019-01-18  5:29   ` Fu, Siyuan
2019-01-18  5:31     ` Wu, Jiaxin
2019-01-18  5:38       ` Gao, Liming
2019-01-18 11:09         ` Ard Biesheuvel
2019-01-19  1:17           ` Laszlo Ersek
2019-01-20 18:25             ` Wu, Jiaxin
2019-01-21 12:53             ` Gao, Liming
2019-01-21 13:26               ` Philippe Mathieu-Daudé
2019-01-21 21:21                 ` Laszlo Ersek
2019-01-21 22:47                   ` Philippe Mathieu-Daudé

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox