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

[Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>]

Yes, we can check return value of NetLibDetectMedia(), then we don't need to initialize it with FALSE in loop.

NetLibDetectMedia() sets the MediaPresent Flag to TRUE in case of success, and don't do anything with it in failure cases.

I will send the patch, let me know if you agreed with this approach.

Thanks,
Meenakshi

> -----Original Message-----
> From: Carsey, Jaben [mailto:jaben.carsey@intel.com]
> Sent: Friday, October 06, 2017 1:57 AM
> 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] Ifconfig : Fixed False information about Media State.
> 
> What are the conditions that the function does not successfully set the
> MediaPresent setting?  Should we also be checking the return value of
> NetLibDetectMedia?
> 
> > -----Original Message-----
> > From: Meenakshi Aggarwal [mailto:meenakshi.aggarwal@nxp.com]
> > Sent: Thursday, October 05, 2017 10:10 AM
> > 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] Ifconfig : Fixed False information about Media State.
> > Importance: High
> >
> > Yes, its moved in the function because NetLibDetectMedia() function is
> > called in a loop (number of active interfaces).
> > So we need to initialize it to FALSE every time before calling
> > NetLibDetectMedia() else if it was set TRUE for some interface then it
> > will remain so for remaining.
> >
> >
> > Thanks,
> > Meenakshi
> >
> > > -----Original Message-----
> > > From: Carsey, Jaben [mailto:jaben.carsey@intel.com]
> > > Sent: Thursday, October 05, 2017 8: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] Ifconfig : Fixed False information about Media State.
> > >
> > > Is there a reason to move the assignment in the function?  I think
> > > our
> > coding
> > > guidelines specify initialize variables up top.
> > >
> > > -Jaben
> > >
> > > > -----Original Message-----
> > > > From: Meenakshi Aggarwal [mailto:meenakshi.aggarwal@nxp.com]
> > > > Sent: Wednesday, October 04, 2017 11:37 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] Ifconfig : Fixed False information about Media State.
> > > > Importance: High
> > > >
> > > > 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 | 4 ++--
> > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git
> > > > a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
> > > > b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
> > > > index 4db07b2..7c05b68 100644
> > > > --- a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
> > > > +++ b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
> > > > @@ -554,8 +554,6 @@ IfConfigShowInterfaceInfo (
> > > >    EFI_IPv4_ADDRESS              Gateway;
> > > >    UINT32                        Index;
> > > >
> > > > -  MediaPresent = TRUE;
> > > > -
> > > >    if (IsListEmpty (IfList)) {
> > > >      ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> > > > (STR_IFCONFIG_INVALID_INTERFACE), gShellNetwork1HiiHandle);
> > > >    }
> > > > @@ -576,6 +574,8 @@ IfConfigShowInterfaceInfo (
> > > >      //
> > > >      // Get Media State.
> > > >      //
> > > > +    MediaPresent = FALSE;
> > > > +
> > > >      NetLibDetectMedia (IfCb->NicHandle, &MediaPresent);
> > > >      if (!MediaPresent) {
> > > >        ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> > > > (STR_IFCONFIG_INFO_MEDIA_STATE), gShellNetwork1HiiHandle,
> > L"Media
> > > > disconnected");
> > > > --
> > > > 2.7.4