Re: [edk2] [PATCH v1] ShellPkg/TftpDynamicCommand: Clarify the retry count option in command.
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 ; Wu, Jiaxin ;
> edk2-devel@lists.01.org
> Cc: Ye, Ting
> 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 ; edk2-devel@lists.01.org
> > Cc: Carsey, Jaben ; Ye, Ting
> > Subject: RE: [PATCH v1] ShellPkg/TftpDynamicCommand: Clarify the retry
> > count option in command.
> > Importance: High
> >
> > Reviewed-by: Fu Siyuan
> >
> >
> >
> > > -Original Message-
> > > From: Wu, Jiaxin
> > > Sent: Monday, November 5, 2018 2:59 PM
> > > To: edk2-devel@lists.01.org
> > > Cc: Carsey, Jaben ; Ye, Ting
> > ;
> > > Fu, Siyuan ; Wu, Jiaxin
> > > Subject: [PATCH v1] ShellPkg/TftpDynamicCommand: Clarify the retry
> > count
> > > option in command.
> > >
> > > [-c ] 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
> > > Cc: Ye Ting
> > > Cc: Fu Siyuan
> > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > Signed-off-by: Wu Jiaxin
> > > ---
> > > 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, )) {
> > >goto Error;
> > > }
> > > +
> > > +if (Mtftp4ConfigData.TryCount == 0) {
> > > + Mtftp4ConfigData.TryCount = 6;
> > > +}
> > >}
> > >
> > >ValueStr = ShellCommandLineGetValue (CheckPackage, L"-t");
> > >if (ValueStr != NULL) {
> > > if (!StringToUint16 (ValueStr, )) {
> > > 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 - 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 - The number of seconds to wait for a response
> > > after\r\n"
> > > " sending a request packet. Default value is 4s.\r\n"
> > > " -s - Specifies the TFTP blksize option as defined in RFC
> > > 2348.\r\n"
> > > "
Re: [edk2] [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 ; edk2-devel@lists.01.org
> Cc: Carsey, Jaben ; Ye, Ting
> Subject: RE: [PATCH v1] ShellPkg/TftpDynamicCommand: Clarify the retry
> count option in command.
> Importance: High
>
> Reviewed-by: Fu Siyuan
>
>
>
> > -Original Message-
> > From: Wu, Jiaxin
> > Sent: Monday, November 5, 2018 2:59 PM
> > To: edk2-devel@lists.01.org
> > Cc: Carsey, Jaben ; Ye, Ting
> ;
> > Fu, Siyuan ; Wu, Jiaxin
> > Subject: [PATCH v1] ShellPkg/TftpDynamicCommand: Clarify the retry
> count
> > option in command.
> >
> > [-c ] 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
> > Cc: Ye Ting
> > Cc: Fu Siyuan
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Wu Jiaxin
> > ---
> > 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, )) {
> >goto Error;
> > }
> > +
> > +if (Mtftp4ConfigData.TryCount == 0) {
> > + Mtftp4ConfigData.TryCount = 6;
> > +}
> >}
> >
> >ValueStr = ShellCommandLineGetValue (CheckPackage, L"-t");
> >if (ValueStr != NULL) {
> > if (!StringToUint16 (ValueStr, )) {
> > 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 - 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 - The number of seconds to wait for a response
> > after\r\n"
> > " sending a request packet. Default value is 4s.\r\n"
> > " -s - 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 - Specifies the TFTP windowsize option as defined in
> > RFC 7440.\r\n"
> > --
> > 2.17.1.windows.2
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v1] ShellPkg/TftpDynamicCommand: Clarify the retry count option in command.
Reviewed-by: Fu Siyuan
> -Original Message-
> From: Wu, Jiaxin
> Sent: Monday, November 5, 2018 2:59 PM
> To: edk2-devel@lists.01.org
> Cc: Carsey, Jaben ; Ye, Ting ;
> Fu, Siyuan ; Wu, Jiaxin
> Subject: [PATCH v1] ShellPkg/TftpDynamicCommand: Clarify the retry count
> option in command.
>
> [-c ] 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
> Cc: Ye Ting
> Cc: Fu Siyuan
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Wu Jiaxin
> ---
> 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, )) {
>goto Error;
> }
> +
> +if (Mtftp4ConfigData.TryCount == 0) {
> + Mtftp4ConfigData.TryCount = 6;
> +}
>}
>
>ValueStr = ShellCommandLineGetValue (CheckPackage, L"-t");
>if (ValueStr != NULL) {
> if (!StringToUint16 (ValueStr, )) {
> 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 - 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 - The number of seconds to wait for a response
> after\r\n"
> " sending a request packet. Default value is 4s.\r\n"
> " -s - 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 - Specifies the TFTP windowsize option as defined in
> RFC 7440.\r\n"
> --
> 2.17.1.windows.2
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v1] ShellPkg/TftpDynamicCommand: Clarify the retry count option in command.
Reviewed-by: Jaben Carsey
> -Original Message-
> From: Wu, Jiaxin
> Sent: Sunday, November 04, 2018 10:59 PM
> To: edk2-devel@lists.01.org
> Cc: Carsey, Jaben ; Ye, Ting ;
> Fu, Siyuan ; Wu, Jiaxin
> Subject: [PATCH v1] ShellPkg/TftpDynamicCommand: Clarify the retry count
> option in command.
> Importance: High
>
> [-c ] 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
> Cc: Ye Ting
> Cc: Fu Siyuan
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Wu Jiaxin
> ---
> 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, )) {
>goto Error;
> }
> +
> +if (Mtftp4ConfigData.TryCount == 0) {
> + Mtftp4ConfigData.TryCount = 6;
> +}
>}
>
>ValueStr = ShellCommandLineGetValue (CheckPackage, L"-t");
>if (ValueStr != NULL) {
> if (!StringToUint16 (ValueStr, )) {
> 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 - 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 - The number of seconds to wait for a response after\r\n"
> " sending a request packet. Default value is 4s.\r\n"
> " -s - 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 - Specifies the TFTP windowsize option as defined in
> RFC 7440.\r\n"
> --
> 2.17.1.windows.2
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v1] ShellPkg/TftpDynamicCommand: Clarify the retry count option in command.
[-c ] 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
Cc: Ye Ting
Cc: Fu Siyuan
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Wu Jiaxin
---
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, )) {
goto Error;
}
+
+if (Mtftp4ConfigData.TryCount == 0) {
+ Mtftp4ConfigData.TryCount = 6;
+}
}
ValueStr = ShellCommandLineGetValue (CheckPackage, L"-t");
if (ValueStr != NULL) {
if (!StringToUint16 (ValueStr, )) {
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 - 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 - The number of seconds to wait for a response after\r\n"
" sending a request packet. Default value is 4s.\r\n"
" -s - 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 - Specifies the TFTP windowsize option as defined in RFC
7440.\r\n"
--
2.17.1.windows.2
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
