Re: [PATCH v3] Ifconfig : Fixed False information about Media State.

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

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