1. Patchew
  2. EDK2
  3. View series

[edk2] [PATCH v2] Ifconfig : Fixed False information about Media State.

Meenakshi Aggarwal posted 1 patch 7 years, 2 months ago
Failed in applying to current master (apply log)
There is a newer version of this series
ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
[edk2] [PATCH v2] Ifconfig : Fixed False information about Media State.
Posted by Meenakshi Aggarwal 7 years, 2 months ago 
Issue : We were setting MediaPresent as TRUE (default), so
even if NetLibDetectMedia() did not set MediaPresent Flag as TRUE,
ifconfig always display Media State as 'Media Present'

Fix : Set MediaPresent as FALSE before calling NetLibDetectMedia()

Contributed-under: TianoCore Contribution Agreement 1.1

Signed-off-by: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>
---
 ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
index 4db07b2..90ca724 100644
--- a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
+++ b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
@@ -576,11 +576,14 @@ IfConfigShowInterfaceInfo (
     //
     // Get Media State.
     //
-    NetLibDetectMedia (IfCb->NicHandle, &MediaPresent);
-    if (!MediaPresent) {
-      ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_IFCONFIG_INFO_MEDIA_STATE), gShellNetwork1HiiHandle, L"Media disconnected");
+    if (EFI_SUCCESS == NetLibDetectMedia (IfCb->NicHandle, &MediaPresent)) {
+      if (!MediaPresent) {
+        ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_IFCONFIG_INFO_MEDIA_STATE), gShellNetwork1HiiHandle, L"Media disconnected");
+      } else {
+        ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_IFCONFIG_INFO_MEDIA_STATE), gShellNetwork1HiiHandle, L"Media present");
+      }
     } else {
-      ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_IFCONFIG_INFO_MEDIA_STATE), gShellNetwork1HiiHandle, L"Media present");
+      ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_IFCONFIG_INFO_MEDIA_STATE), gShellNetwork1HiiHandle, L"Media disconnected");
     }
 
     //
-- 
2.7.4

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v3] *** Ifconfig : Fixed False information about Media State ***
Posted by Meenakshi Aggarwal 7 years, 2 months ago 
Corrected commit message

Meenakshi Aggarwal (1):
  Ifconfig : Fixed False information about Media State.

 ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

-- 
2.7.4

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v3] Ifconfig : Fixed False information about Media State.
Posted by Meenakshi Aggarwal 7 years, 2 months ago 
Issue : We were setting MediaPresent as TRUE (default) and
not checking return status of NetLibDetectMedia().
NetLibDetectMedia() sets MediaPresent FLAG in case of success
only and dont change flag on error.
So, Media State will display as 'Media Present', in case of
error also.

Fix : Check return value of NetLibDetectMedia()

Contributed-under: TianoCore Contribution Agreement 1.1

Signed-off-by: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>
---
 ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
index 4db07b2..90ca724 100644
--- a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
+++ b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
@@ -576,11 +576,14 @@ IfConfigShowInterfaceInfo (
     //
     // Get Media State.
     //
-    NetLibDetectMedia (IfCb->NicHandle, &MediaPresent);
-    if (!MediaPresent) {
-      ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_IFCONFIG_INFO_MEDIA_STATE), gShellNetwork1HiiHandle, L"Media disconnected");
+    if (EFI_SUCCESS == NetLibDetectMedia (IfCb->NicHandle, &MediaPresent)) {
+      if (!MediaPresent) {
+        ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_IFCONFIG_INFO_MEDIA_STATE), gShellNetwork1HiiHandle, L"Media disconnected");
+      } else {
+        ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_IFCONFIG_INFO_MEDIA_STATE), gShellNetwork1HiiHandle, L"Media present");
+      }
     } else {
-      ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_IFCONFIG_INFO_MEDIA_STATE), gShellNetwork1HiiHandle, L"Media present");
+      ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_IFCONFIG_INFO_MEDIA_STATE), gShellNetwork1HiiHandle, L"Media disconnected");
     }
 
     //
-- 
2.7.4

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v3] Ifconfig : Fixed False information about Media State.
Posted by Carsey, Jaben 7 years, 2 months ago 
Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>
Do you know under what conditions the API will fail? Is it worth saying something like media stats unknown when the function fails?

Ray,

What do you think?


> -----Original Message-----
> From: Meenakshi Aggarwal [mailto:meenakshi.aggarwal@nxp.com]
> Sent: Thursday, October 05, 2017 9:48 PM
> To: edk2-devel@lists.01.org; Wu, Jiaxin <jiaxin.wu@intel.com>; Carsey,
> Jaben <jaben.carsey@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>
> Cc: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>
> Subject: [PATCH v3] Ifconfig : Fixed False information about Media State.
> Importance: High
> 
> Issue : We were setting MediaPresent as TRUE (default) and
> not checking return status of NetLibDetectMedia().
> NetLibDetectMedia() sets MediaPresent FLAG in case of success
> only and dont change flag on error.
> So, Media State will display as 'Media Present', in case of
> error also.
> 
> Fix : Check return value of NetLibDetectMedia()
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> 
> Signed-off-by: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>
> ---
>  ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c | 11 +++++++--
> --
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
> b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
> index 4db07b2..90ca724 100644
> --- a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
> +++ b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
> @@ -576,11 +576,14 @@ IfConfigShowInterfaceInfo (
>      //
>      // Get Media State.
>      //
> -    NetLibDetectMedia (IfCb->NicHandle, &MediaPresent);
> -    if (!MediaPresent) {
> -      ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> (STR_IFCONFIG_INFO_MEDIA_STATE), gShellNetwork1HiiHandle, L"Media
> disconnected");
> +    if (EFI_SUCCESS == NetLibDetectMedia (IfCb->NicHandle,
> &MediaPresent)) {
> +      if (!MediaPresent) {
> +        ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> (STR_IFCONFIG_INFO_MEDIA_STATE), gShellNetwork1HiiHandle, L"Media
> disconnected");
> +      } else {
> +        ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> (STR_IFCONFIG_INFO_MEDIA_STATE), gShellNetwork1HiiHandle, L"Media
> present");
> +      }
>      } else {
> -      ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> (STR_IFCONFIG_INFO_MEDIA_STATE), gShellNetwork1HiiHandle, L"Media
> present");
> +      ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> (STR_IFCONFIG_INFO_MEDIA_STATE), gShellNetwork1HiiHandle, L"Media
> disconnected");
>      }
> 
>      //
> --
> 2.7.4

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v3] Ifconfig : Fixed False information about Media State.
Posted by Meenakshi Aggarwal 7 years, 2 months ago 
It is hard to say when can an API fail because its dependent on implementation.


> -----Original Message-----
> From: Carsey, Jaben [mailto:jaben.carsey@intel.com]
> Sent: Friday, October 06, 2017 7:32 PM
> To: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>; edk2-
> devel@lists.01.org; Wu, Jiaxin <jiaxin.wu@intel.com>; Ni, Ruiyu
> <ruiyu.ni@intel.com>
> Subject: RE: [PATCH v3] Ifconfig : Fixed False information about Media State.
> 
> Reviewed-by: Jaben Carsey <jaben.carsey@intel.com> Do you know under
> what conditions the API will fail? Is it worth saying something like media stats
> unknown when the function fails?
> 
> Ray,
> 
> What do you think?
> 
> 
> > -----Original Message-----
> > From: Meenakshi Aggarwal [mailto:meenakshi.aggarwal@nxp.com]
> > Sent: Thursday, October 05, 2017 9:48 PM
> > To: edk2-devel@lists.01.org; Wu, Jiaxin <jiaxin.wu@intel.com>; Carsey,
> > Jaben <jaben.carsey@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>
> > Cc: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>
> > Subject: [PATCH v3] Ifconfig : Fixed False information about Media State.
> > Importance: High
> >
> > Issue : We were setting MediaPresent as TRUE (default) and not
> > checking return status of NetLibDetectMedia().
> > NetLibDetectMedia() sets MediaPresent FLAG in case of success only and
> > dont change flag on error.
> > So, Media State will display as 'Media Present', in case of error
> > also.
> >
> > Fix : Check return value of NetLibDetectMedia()
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> >
> > Signed-off-by: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>
> > ---
> >  ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c | 11
> > +++++++--
> > --
> >  1 file changed, 7 insertions(+), 4 deletions(-)
> >
> > diff --git a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
> > b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
> > index 4db07b2..90ca724 100644
> > --- a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
> > +++ b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
> > @@ -576,11 +576,14 @@ IfConfigShowInterfaceInfo (
> >      //
> >      // Get Media State.
> >      //
> > -    NetLibDetectMedia (IfCb->NicHandle, &MediaPresent);
> > -    if (!MediaPresent) {
> > -      ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> > (STR_IFCONFIG_INFO_MEDIA_STATE), gShellNetwork1HiiHandle, L"Media
> > disconnected");
> > +    if (EFI_SUCCESS == NetLibDetectMedia (IfCb->NicHandle,
> > &MediaPresent)) {
> > +      if (!MediaPresent) {
> > +        ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> > (STR_IFCONFIG_INFO_MEDIA_STATE), gShellNetwork1HiiHandle, L"Media
> > disconnected");
> > +      } else {
> > +        ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> > (STR_IFCONFIG_INFO_MEDIA_STATE), gShellNetwork1HiiHandle, L"Media
> > present");
> > +      }
> >      } else {
> > -      ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> > (STR_IFCONFIG_INFO_MEDIA_STATE), gShellNetwork1HiiHandle, L"Media
> > present");
> > +      ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> > (STR_IFCONFIG_INFO_MEDIA_STATE), gShellNetwork1HiiHandle, L"Media
> > disconnected");
> >      }
> >
> >      //
> > --
> > 2.7.4

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v3] Ifconfig : Fixed False information about Media State.
Posted by Wu, Jiaxin 7 years, 2 months ago 
I agree with Jaben. If NetLibDetectMedia return error status, we can output as below:

      ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_IFCONFIG_INFO_MEDIA_STATE), gShellNetwork1HiiHandle, L"Media state unknown");

Thanks,
Jiaxin

> -----Original Message-----
> From: Meenakshi Aggarwal [mailto:meenakshi.aggarwal@nxp.com]
> Sent: Sunday, October 8, 2017 4:49 PM
> To: Carsey, Jaben <jaben.carsey@intel.com>; edk2-devel@lists.01.org; Wu,
> Jiaxin <jiaxin.wu@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>
> Subject: RE: [PATCH v3] Ifconfig : Fixed False information about Media State.
> 
> It is hard to say when can an API fail because its dependent on
> implementation.
> 
> 
> > -----Original Message-----
> > From: Carsey, Jaben [mailto:jaben.carsey@intel.com]
> > Sent: Friday, October 06, 2017 7:32 PM
> > To: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>; edk2-
> > devel@lists.01.org; Wu, Jiaxin <jiaxin.wu@intel.com>; Ni, Ruiyu
> > <ruiyu.ni@intel.com>
> > Subject: RE: [PATCH v3] Ifconfig : Fixed False information about Media
> State.
> >
> > Reviewed-by: Jaben Carsey <jaben.carsey@intel.com> Do you know under
> > what conditions the API will fail? Is it worth saying something like media
> stats
> > unknown when the function fails?
> >
> > Ray,
> >
> > What do you think?
> >
> >
> > > -----Original Message-----
> > > From: Meenakshi Aggarwal [mailto:meenakshi.aggarwal@nxp.com]
> > > Sent: Thursday, October 05, 2017 9:48 PM
> > > To: edk2-devel@lists.01.org; Wu, Jiaxin <jiaxin.wu@intel.com>; Carsey,
> > > Jaben <jaben.carsey@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>
> > > Cc: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>
> > > Subject: [PATCH v3] Ifconfig : Fixed False information about Media State.
> > > Importance: High
> > >
> > > Issue : We were setting MediaPresent as TRUE (default) and not
> > > checking return status of NetLibDetectMedia().
> > > NetLibDetectMedia() sets MediaPresent FLAG in case of success only and
> > > dont change flag on error.
> > > So, Media State will display as 'Media Present', in case of error
> > > also.
> > >
> > > Fix : Check return value of NetLibDetectMedia()
> > >
> > > Contributed-under: TianoCore Contribution Agreement 1.1
> > >
> > > Signed-off-by: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>
> > > ---
> > >  ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c | 11
> > > +++++++--
> > > --
> > >  1 file changed, 7 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
> > > b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
> > > index 4db07b2..90ca724 100644
> > > --- a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
> > > +++ b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
> > > @@ -576,11 +576,14 @@ IfConfigShowInterfaceInfo (
> > >      //
> > >      // Get Media State.
> > >      //
> > > -    NetLibDetectMedia (IfCb->NicHandle, &MediaPresent);
> > > -    if (!MediaPresent) {
> > > -      ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> > > (STR_IFCONFIG_INFO_MEDIA_STATE), gShellNetwork1HiiHandle,
> L"Media
> > > disconnected");
> > > +    if (EFI_SUCCESS == NetLibDetectMedia (IfCb->NicHandle,
> > > &MediaPresent)) {
> > > +      if (!MediaPresent) {
> > > +        ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> > > (STR_IFCONFIG_INFO_MEDIA_STATE), gShellNetwork1HiiHandle,
> L"Media
> > > disconnected");
> > > +      } else {
> > > +        ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> > > (STR_IFCONFIG_INFO_MEDIA_STATE), gShellNetwork1HiiHandle,
> L"Media
> > > present");
> > > +      }
> > >      } else {
> > > -      ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> > > (STR_IFCONFIG_INFO_MEDIA_STATE), gShellNetwork1HiiHandle,
> L"Media
> > > present");
> > > +      ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> > > (STR_IFCONFIG_INFO_MEDIA_STATE), gShellNetwork1HiiHandle,
> L"Media
> > > disconnected");
> > >      }
> > >
> > >      //
> > > --
> > > 2.7.4

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v3] Ifconfig : Fixed False information about Media State.
Posted by Ni, Ruiyu 7 years, 2 months ago 
Do you need to put all the hard-code string in UNI file for localization?

Thanks/Ray

> -----Original Message-----
> From: Wu, Jiaxin
> Sent: Monday, October 9, 2017 9:29 AM
> To: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>; Carsey, Jaben
> <jaben.carsey@intel.com>; edk2-devel@lists.01.org; Ni, Ruiyu
> <ruiyu.ni@intel.com>
> Subject: RE: [PATCH v3] Ifconfig : Fixed False information about Media State.
> 
> I agree with Jaben. If NetLibDetectMedia return error status, we can output as
> below:
> 
>       ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> (STR_IFCONFIG_INFO_MEDIA_STATE), gShellNetwork1HiiHandle, L"Media state
> unknown");
> 
> Thanks,
> Jiaxin
> 
> > -----Original Message-----
> > From: Meenakshi Aggarwal [mailto:meenakshi.aggarwal@nxp.com]
> > Sent: Sunday, October 8, 2017 4:49 PM
> > To: Carsey, Jaben <jaben.carsey@intel.com>; edk2-devel@lists.01.org;
> > Wu, Jiaxin <jiaxin.wu@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>
> > Subject: RE: [PATCH v3] Ifconfig : Fixed False information about Media State.
> >
> > It is hard to say when can an API fail because its dependent on
> > implementation.
> >
> >
> > > -----Original Message-----
> > > From: Carsey, Jaben [mailto:jaben.carsey@intel.com]
> > > Sent: Friday, October 06, 2017 7:32 PM
> > > To: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>; edk2-
> > > devel@lists.01.org; Wu, Jiaxin <jiaxin.wu@intel.com>; Ni, Ruiyu
> > > <ruiyu.ni@intel.com>
> > > Subject: RE: [PATCH v3] Ifconfig : Fixed False information about
> > > Media
> > State.
> > >
> > > Reviewed-by: Jaben Carsey <jaben.carsey@intel.com> Do you know under
> > > what conditions the API will fail? Is it worth saying something like
> > > media
> > stats
> > > unknown when the function fails?
> > >
> > > Ray,
> > >
> > > What do you think?
> > >
> > >
> > > > -----Original Message-----
> > > > From: Meenakshi Aggarwal [mailto:meenakshi.aggarwal@nxp.com]
> > > > Sent: Thursday, October 05, 2017 9:48 PM
> > > > To: edk2-devel@lists.01.org; Wu, Jiaxin <jiaxin.wu@intel.com>;
> > > > Carsey, Jaben <jaben.carsey@intel.com>; Ni, Ruiyu
> > > > <ruiyu.ni@intel.com>
> > > > Cc: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>
> > > > Subject: [PATCH v3] Ifconfig : Fixed False information about Media State.
> > > > Importance: High
> > > >
> > > > Issue : We were setting MediaPresent as TRUE (default) and not
> > > > checking return status of NetLibDetectMedia().
> > > > NetLibDetectMedia() sets MediaPresent FLAG in case of success only
> > > > and dont change flag on error.
> > > > So, Media State will display as 'Media Present', in case of error
> > > > also.
> > > >
> > > > Fix : Check return value of NetLibDetectMedia()
> > > >
> > > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > >
> > > > Signed-off-by: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>
> > > > ---
> > > >  ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c | 11
> > > > +++++++--
> > > > --
> > > >  1 file changed, 7 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git
> > > > a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
> > > > b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
> > > > index 4db07b2..90ca724 100644
> > > > --- a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
> > > > +++ b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
> > > > @@ -576,11 +576,14 @@ IfConfigShowInterfaceInfo (
> > > >      //
> > > >      // Get Media State.
> > > >      //
> > > > -    NetLibDetectMedia (IfCb->NicHandle, &MediaPresent);
> > > > -    if (!MediaPresent) {
> > > > -      ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> > > > (STR_IFCONFIG_INFO_MEDIA_STATE), gShellNetwork1HiiHandle,
> > L"Media
> > > > disconnected");
> > > > +    if (EFI_SUCCESS == NetLibDetectMedia (IfCb->NicHandle,
> > > > &MediaPresent)) {
> > > > +      if (!MediaPresent) {
> > > > +        ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> > > > (STR_IFCONFIG_INFO_MEDIA_STATE), gShellNetwork1HiiHandle,
> > L"Media
> > > > disconnected");
> > > > +      } else {
> > > > +        ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> > > > (STR_IFCONFIG_INFO_MEDIA_STATE), gShellNetwork1HiiHandle,
> > L"Media
> > > > present");
> > > > +      }
> > > >      } else {
> > > > -      ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> > > > (STR_IFCONFIG_INFO_MEDIA_STATE), gShellNetwork1HiiHandle,
> > L"Media
> > > > present");
> > > > +      ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> > > > (STR_IFCONFIG_INFO_MEDIA_STATE), gShellNetwork1HiiHandle,
> > L"Media
> > > > disconnected");
> > > >      }
> > > >
> > > >      //
> > > > --
> > > > 2.7.4

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v3] Ifconfig : Fixed False information about Media State.
Posted by Wu, Jiaxin 7 years, 2 months ago 
For me, the hard-code string looks good to me here. I'm also fine if you stick to move in UNI file.

Thanks,
Jiaxin 


> -----Original Message-----
> From: Ni, Ruiyu
> Sent: Monday, October 9, 2017 10:09 AM
> To: Wu, Jiaxin <jiaxin.wu@intel.com>; Meenakshi Aggarwal
> <meenakshi.aggarwal@nxp.com>; Carsey, Jaben <jaben.carsey@intel.com>;
> edk2-devel@lists.01.org
> Subject: RE: [PATCH v3] Ifconfig : Fixed False information about Media State.
> 
> Do you need to put all the hard-code string in UNI file for localization?
> 
> Thanks/Ray
> 
> > -----Original Message-----
> > From: Wu, Jiaxin
> > Sent: Monday, October 9, 2017 9:29 AM
> > To: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>; Carsey, Jaben
> > <jaben.carsey@intel.com>; edk2-devel@lists.01.org; Ni, Ruiyu
> > <ruiyu.ni@intel.com>
> > Subject: RE: [PATCH v3] Ifconfig : Fixed False information about Media
> State.
> >
> > I agree with Jaben. If NetLibDetectMedia return error status, we can
> output as
> > below:
> >
> >       ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> > (STR_IFCONFIG_INFO_MEDIA_STATE), gShellNetwork1HiiHandle, L"Media
> state
> > unknown");
> >
> > Thanks,
> > Jiaxin
> >
> > > -----Original Message-----
> > > From: Meenakshi Aggarwal [mailto:meenakshi.aggarwal@nxp.com]
> > > Sent: Sunday, October 8, 2017 4:49 PM
> > > To: Carsey, Jaben <jaben.carsey@intel.com>; edk2-devel@lists.01.org;
> > > Wu, Jiaxin <jiaxin.wu@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>
> > > Subject: RE: [PATCH v3] Ifconfig : Fixed False information about Media
> State.
> > >
> > > It is hard to say when can an API fail because its dependent on
> > > implementation.
> > >
> > >
> > > > -----Original Message-----
> > > > From: Carsey, Jaben [mailto:jaben.carsey@intel.com]
> > > > Sent: Friday, October 06, 2017 7:32 PM
> > > > To: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>; edk2-
> > > > devel@lists.01.org; Wu, Jiaxin <jiaxin.wu@intel.com>; Ni, Ruiyu
> > > > <ruiyu.ni@intel.com>
> > > > Subject: RE: [PATCH v3] Ifconfig : Fixed False information about
> > > > Media
> > > State.
> > > >
> > > > Reviewed-by: Jaben Carsey <jaben.carsey@intel.com> Do you know
> under
> > > > what conditions the API will fail? Is it worth saying something like
> > > > media
> > > stats
> > > > unknown when the function fails?
> > > >
> > > > Ray,
> > > >
> > > > What do you think?
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Meenakshi Aggarwal [mailto:meenakshi.aggarwal@nxp.com]
> > > > > Sent: Thursday, October 05, 2017 9:48 PM
> > > > > To: edk2-devel@lists.01.org; Wu, Jiaxin <jiaxin.wu@intel.com>;
> > > > > Carsey, Jaben <jaben.carsey@intel.com>; Ni, Ruiyu
> > > > > <ruiyu.ni@intel.com>
> > > > > Cc: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>
> > > > > Subject: [PATCH v3] Ifconfig : Fixed False information about Media
> State.
> > > > > Importance: High
> > > > >
> > > > > Issue : We were setting MediaPresent as TRUE (default) and not
> > > > > checking return status of NetLibDetectMedia().
> > > > > NetLibDetectMedia() sets MediaPresent FLAG in case of success only
> > > > > and dont change flag on error.
> > > > > So, Media State will display as 'Media Present', in case of error
> > > > > also.
> > > > >
> > > > > Fix : Check return value of NetLibDetectMedia()
> > > > >
> > > > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > > >
> > > > > Signed-off-by: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>
> > > > > ---
> > > > >  ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c | 11
> > > > > +++++++--
> > > > > --
> > > > >  1 file changed, 7 insertions(+), 4 deletions(-)
> > > > >
> > > > > diff --git
> > > > > a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
> > > > > b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
> > > > > index 4db07b2..90ca724 100644
> > > > > --- a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
> > > > > +++ b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
> > > > > @@ -576,11 +576,14 @@ IfConfigShowInterfaceInfo (
> > > > >      //
> > > > >      // Get Media State.
> > > > >      //
> > > > > -    NetLibDetectMedia (IfCb->NicHandle, &MediaPresent);
> > > > > -    if (!MediaPresent) {
> > > > > -      ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> > > > > (STR_IFCONFIG_INFO_MEDIA_STATE), gShellNetwork1HiiHandle,
> > > L"Media
> > > > > disconnected");
> > > > > +    if (EFI_SUCCESS == NetLibDetectMedia (IfCb->NicHandle,
> > > > > &MediaPresent)) {
> > > > > +      if (!MediaPresent) {
> > > > > +        ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> > > > > (STR_IFCONFIG_INFO_MEDIA_STATE), gShellNetwork1HiiHandle,
> > > L"Media
> > > > > disconnected");
> > > > > +      } else {
> > > > > +        ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> > > > > (STR_IFCONFIG_INFO_MEDIA_STATE), gShellNetwork1HiiHandle,
> > > L"Media
> > > > > present");
> > > > > +      }
> > > > >      } else {
> > > > > -      ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> > > > > (STR_IFCONFIG_INFO_MEDIA_STATE), gShellNetwork1HiiHandle,
> > > L"Media
> > > > > present");
> > > > > +      ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> > > > > (STR_IFCONFIG_INFO_MEDIA_STATE), gShellNetwork1HiiHandle,
> > > L"Media
> > > > > disconnected");
> > > > >      }
> > > > >
> > > > >      //
> > > > > --
> > > > > 2.7.4

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v4] Ifconfig : Fixed False information about Media State.
Posted by Meenakshi Aggarwal 7 years, 2 months ago 
Issue : We were setting MediaPresent as TRUE (default) and
not checking return status of NetLibDetectMedia().
NetLibDetectMedia() sets MediaPresent FLAG in case of success
only and dont change flag on error.
So, Media State will display as 'Media Present', in case of
error also.

Fix : Check return value of NetLibDetectMedia(), if error then
print "Media State Unknown"

Contributed-under: TianoCore Contribution Agreement 1.1

Signed-off-by: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>
---
 ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
index 4db07b2..082ab72 100644
--- a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
+++ b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
@@ -576,11 +576,14 @@ IfConfigShowInterfaceInfo (
     //
     // Get Media State.
     //
-    NetLibDetectMedia (IfCb->NicHandle, &MediaPresent);
-    if (!MediaPresent) {
-      ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_IFCONFIG_INFO_MEDIA_STATE), gShellNetwork1HiiHandle, L"Media disconnected");
+    if (EFI_SUCCESS == NetLibDetectMedia (IfCb->NicHandle, &MediaPresent)) {
+      if (!MediaPresent) {
+        ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_IFCONFIG_INFO_MEDIA_STATE), gShellNetwork1HiiHandle, L"Media disconnected");
+      } else {
+        ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_IFCONFIG_INFO_MEDIA_STATE), gShellNetwork1HiiHandle, L"Media present");
+      }
     } else {
-      ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_IFCONFIG_INFO_MEDIA_STATE), gShellNetwork1HiiHandle, L"Media present");
+      ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_IFCONFIG_INFO_MEDIA_STATE), gShellNetwork1HiiHandle, L"Media state unknown");
     }
 
     //
-- 
2.7.4

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v4] Ifconfig : Fixed False information about Media State.
Posted by Wu, Jiaxin 7 years, 2 months ago 
Reviewed-by: Wu Jiaxin <jiaxin.wu@intel.com>


> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Meenakshi Aggarwal
> Sent: Tuesday, October 10, 2017 10:08 PM
> To: edk2-devel@lists.01.org; Wu, Jiaxin <jiaxin.wu@intel.com>; Carsey,
> Jaben <jaben.carsey@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>
> Subject: [edk2] [PATCH v4] Ifconfig : Fixed False information about Media
> State.
> 
> Issue : We were setting MediaPresent as TRUE (default) and
> not checking return status of NetLibDetectMedia().
> NetLibDetectMedia() sets MediaPresent FLAG in case of success
> only and dont change flag on error.
> So, Media State will display as 'Media Present', in case of
> error also.
> 
> Fix : Check return value of NetLibDetectMedia(), if error then
> print "Media State Unknown"
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> 
> Signed-off-by: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>
> ---
>  ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c | 11 +++++++--
> --
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
> b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
> index 4db07b2..082ab72 100644
> --- a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
> +++ b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
> @@ -576,11 +576,14 @@ IfConfigShowInterfaceInfo (
>      //
>      // Get Media State.
>      //
> -    NetLibDetectMedia (IfCb->NicHandle, &MediaPresent);
> -    if (!MediaPresent) {
> -      ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> (STR_IFCONFIG_INFO_MEDIA_STATE), gShellNetwork1HiiHandle, L"Media
> disconnected");
> +    if (EFI_SUCCESS == NetLibDetectMedia (IfCb->NicHandle,
> &MediaPresent)) {
> +      if (!MediaPresent) {
> +        ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> (STR_IFCONFIG_INFO_MEDIA_STATE), gShellNetwork1HiiHandle, L"Media
> disconnected");
> +      } else {
> +        ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> (STR_IFCONFIG_INFO_MEDIA_STATE), gShellNetwork1HiiHandle, L"Media
> present");
> +      }
>      } else {
> -      ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> (STR_IFCONFIG_INFO_MEDIA_STATE), gShellNetwork1HiiHandle, L"Media
> present");
> +      ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> (STR_IFCONFIG_INFO_MEDIA_STATE), gShellNetwork1HiiHandle, L"Media
> state unknown");
>      }
> 
>      //
> --
> 2.7.4
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Patchew 2.1-devel-8c64711

© 2016 - 2024 Red Hat, Inc.