Re: [PATCH v1] ShellPkg/TftpDynamicCommand: Clarify the retry count option in command.

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

Hi Jaben,

The patch already has been pushed after received your/Siyuan reviewed-by tag.

Thanks,
Jiaxin


> -----Original Message-----
> From: Carsey, Jaben
> Sent: Thursday, November 8, 2018 9:09 AM
> To: Fu, Siyuan <siyuan.fu@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>;
> edk2-devel@lists.01.org
> Cc: Ye, Ting <ting.ye@intel.com>
> Subject: RE: [PATCH v1] ShellPkg/TftpDynamicCommand: Clarify the retry
> count option in command.
> 
> Wu,
> 
> I plan to push this patch tomorrow, but I would like to add this to the commit
> message.  What do you think?
> 
> "This fixes the bug where parameter value 0 causes failure."
> 
> -Jaben
> 
> > -----Original Message-----
> > From: Fu, Siyuan
> > Sent: Monday, November 05, 2018 10:51 PM
> > To: Wu, Jiaxin <jiaxin.wu@intel.com>; edk2-devel@lists.01.org
> > Cc: Carsey, Jaben <jaben.carsey@intel.com>; Ye, Ting <ting.ye@intel.com>
> > Subject: RE: [PATCH v1] ShellPkg/TftpDynamicCommand: Clarify the retry
> > count option in command.
> > Importance: High
> >
> > Reviewed-by: Fu Siyuan <siyuan.fu@intel.com>
> >
> >
> >
> > > -----Original Message-----
> > > From: Wu, Jiaxin
> > > Sent: Monday, November 5, 2018 2:59 PM
> > > To: edk2-devel@lists.01.org
> > > Cc: Carsey, Jaben <jaben.carsey@intel.com>; Ye, Ting
> > <ting.ye@intel.com>;
> > > Fu, Siyuan <siyuan.fu@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>
> > > Subject: [PATCH v1] ShellPkg/TftpDynamicCommand: Clarify the retry
> > count
> > > option in command.
> > >
> > > [-c <retry count>] is to define the number of times to transmit request
> > > packets and wait for a response. The default value is 6. But it doesn't
> > > specify the behavior of zero value. Here, The patch is to clear that:
> > > Set to zero also means to use the default value.
> > >
> > > Cc: Carsey Jaben <jaben.carsey@intel.com>
> > > Cc: Ye Ting <ting.ye@intel.com>
> > > Cc: Fu Siyuan <siyuan.fu@intel.com>
> > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
> > > ---
> > >  ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.c   | 6 +++++-
> > >  ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.uni | 3 ++-
> > >  2 files changed, 7 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.c
> > > b/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.c
> > > index ac2813efc3..028686e1ff 100644
> > > --- a/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.c
> > > +++ b/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.c
> > > @@ -216,11 +216,11 @@ EFI_MTFTP4_CONFIG_DATA
> > DefaultMtftp4ConfigData = {
> > >    { { 0, 0, 0, 0 } },               // SubnetMask        - Not relevant
> > > as UseDefaultSetting=TRUE
> > >    0,                                // LocalPort         - Automatically
> > > assigned port number.
> > >    { { 0, 0, 0, 0 } },               // GatewayIp         - Not relevant
> > > as UseDefaultSetting=TRUE
> > >    { { 0, 0, 0, 0 } },               // ServerIp          - Not known yet
> > >    69,                               // InitialServerPort - Standard TFTP
> > > server port
> > > -  6,                                // TryCount          - Max number of
> > > retransmissions.
> > > +  6,                                // TryCount          - The number of
> > > times to transmit request packets and wait for a response.
> > >    4                                 // TimeoutValue      - Retransmission
> > > timeout in seconds.
> > >  };
> > >
> > >  STATIC CONST SHELL_PARAM_ITEM ParamList[] = {
> > >    {L"-i", TypeValue},
> > > @@ -419,10 +419,14 @@ RunTftp (
> > >    ValueStr = ShellCommandLineGetValue (CheckPackage, L"-c");
> > >    if (ValueStr != NULL) {
> > >      if (!StringToUint16 (ValueStr, &Mtftp4ConfigData.TryCount)) {
> > >        goto Error;
> > >      }
> > > +
> > > +    if (Mtftp4ConfigData.TryCount == 0) {
> > > +      Mtftp4ConfigData.TryCount = 6;
> > > +    }
> > >    }
> > >
> > >    ValueStr = ShellCommandLineGetValue (CheckPackage, L"-t");
> > >    if (ValueStr != NULL) {
> > >      if (!StringToUint16 (ValueStr, &Mtftp4ConfigData.TimeoutValue)) {
> > > diff --git a/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.uni
> > > b/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.uni
> > > index 654e42ad23..ff64912564 100644
> > > --- a/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.uni
> > > +++ b/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.uni
> > > @@ -56,11 +56,12 @@
> > >  "  -i interface     - Specifies an adapter name, i.e., eth0.\r\n"
> > >  "  -l port          - Specifies the local port number. Default value is
> > > 0\r\n"
> > >  "                     and the port number is automatically assigned.\r\n"
> > >  "  -r port          - Specifies the remote port number. Default value is
> > > 69.\r\n"
> > >  "  -c <retry count> - The number of times to transmit request packets
> > > and\r\n"
> > > -"                     wait for a response. The default value is 6.\r\n"
> > > +"                     wait for a response. The default value is 6. Set to
> > > zero\r\n"
> > > +"                     also means to use the default value.\r\n"
> > >  "  -t <timeout>     - The number of seconds to wait for a response
> > > after\r\n"
> > >  "                     sending a request packet. Default value is 4s.\r\n"
> > >  "  -s <block size>  - Specifies the TFTP blksize option as defined in RFC
> > > 2348.\r\n"
> > >  "                     Valid range is between 8 and 65464, default value
> > > is 512.\r\n"
> > >  "  -w <window size> - Specifies the TFTP windowsize option as defined in
> > > RFC 7440.\r\n"
> > > --
> > > 2.17.1.windows.2