Re: [edk2] [PATCH] ShellPkg: Fix the wrong value got from 'for' command.

2015-07-15 Thread Leif Lindholm 
On Wed, Jul 15, 2015 at 09:40:12AM +0800, Qiu Shumin wrote:
> In 'For.c' return error status instead of a wrong number when fail to convert 
> a string to UINT64.

No objection to content of patch, but the above line is too long.
Can you break it up please?

Regards,

Leif

> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jaben Carsey 
> Reviewed-by: Qiu Shumin 
> ---
>  ShellPkg/Library/UefiShellLevel1CommandsLib/For.c | 53 
> ---
>  1 file changed, 18 insertions(+), 35 deletions(-)
> 
> diff --git a/ShellPkg/Library/UefiShellLevel1CommandsLib/For.c 
> b/ShellPkg/Library/UefiShellLevel1CommandsLib/For.c
> index 2ecc5cd..3a6852e 100644
> --- a/ShellPkg/Library/UefiShellLevel1CommandsLib/For.c
> +++ b/ShellPkg/Library/UefiShellLevel1CommandsLib/For.c
> @@ -2,7 +2,7 @@
>Main file for endfor and for shell level 1 functions.
>  
>(C) Copyright 2015 Hewlett-Packard Development Company, L.P.
> -  Copyright (c) 2009 - 2014, Intel Corporation. All rights reserved.
> +  Copyright (c) 2009 - 2015, Intel Corporation. All rights reserved.
>This program and the accompanying materials
>are licensed and made available under the terms and conditions of the BSD 
> License
>which accompanies this distribution.  The full text of the license may be 
> found at
> @@ -255,29 +255,6 @@ InternalRemoveAliasFromList(
>  }
>  
>  /**
> -  Function to determine whether a string is decimal or hex representation of 
> a number
> -  and return the number converted from the string.
> -
> -  @param[in] String   String representation of a number
> -
> -  @return the number
> -  @retval (UINTN)(-1) An error ocurred.
> -**/
> -UINTN
> -EFIAPI
> -ReturnUintn(
> -  IN CONST CHAR16 *String
> -  )
> -{
> -  UINT64RetVal;
> -
> -  if (!EFI_ERROR(ShellConvertStringToUint64(String, &RetVal, FALSE, TRUE))) {
> -return ((UINTN)RetVal);
> -  }
> -  return ((UINTN)(-1));
> -}
> -
> -/**
>Function for 'for' command.
>  
>@param[in] ImageHandle  Handle to the Image (NULL if Internal).
> @@ -305,6 +282,7 @@ ShellCommandRunFor (
>EFI_SHELL_FILE_INFO *Node;
>EFI_SHELL_FILE_INFO *FileList;
>UINTN   NewSize;
> +  UINT64  Temp;
>  
>ArgSet  = NULL;
>ArgSize = 0;
> @@ -509,15 +487,17 @@ ShellCommandRunFor (
>ShellStatus = SHELL_INVALID_PARAMETER;
>  } else {
>if (ArgSetWalker[0] == L'-') {
> -Info->Current = 0 - (INTN)ReturnUintn(ArgSetWalker+1);
> +Status = ShellConvertStringToUint64(ArgSetWalker+1, &Temp, 
> FALSE, TRUE);
> +Info->Current = 0 - (INTN)Temp;
>} else {
> -Info->Current = (INTN)ReturnUintn(ArgSetWalker);
> +Status = ShellConvertStringToUint64(ArgSetWalker, &Temp, 
> FALSE, TRUE);
> +Info->Current = (INTN)Temp;
>}
>ArgSetWalker  = StrStr(ArgSetWalker, L" ");
>while (ArgSetWalker != NULL && ArgSetWalker[0] == L' ') {
>  ArgSetWalker++;
>}
> -  if (ArgSetWalker == NULL || *ArgSetWalker == CHAR_NULL || 
> !ShellIsValidForNumber(ArgSetWalker)){
> +  if (EFI_ERROR(Status) || ArgSetWalker == NULL || *ArgSetWalker 
> == CHAR_NULL || !ShellIsValidForNumber(ArgSetWalker)){
>  ShellPrintHiiEx(
>-1, 
>-1, 
> @@ -529,9 +509,11 @@ ShellCommandRunFor (
>  ShellStatus = SHELL_INVALID_PARAMETER;
>} else {
>  if (ArgSetWalker[0] == L'-') {
> -  Info->End = 0 - (INTN)ReturnUintn(ArgSetWalker+1);
> +  Status = ShellConvertStringToUint64(ArgSetWalker+1, &Temp, 
> FALSE, TRUE);
> +  Info->End = 0 - (INTN)Temp;
>  } else {
> -  Info->End = (INTN)ReturnUintn(ArgSetWalker);
> +  Status = ShellConvertStringToUint64(ArgSetWalker, &Temp, 
> FALSE, TRUE);
> +  Info->End = (INTN)Temp;
>  }
>  if (Info->Current < Info->End) {
>Info->Step= 1;
> @@ -543,8 +525,8 @@ ShellCommandRunFor (
>  while (ArgSetWalker != NULL && ArgSetWalker[0] == L' ') {
>ArgSetWalker++;
>  }
> -if (ArgSetWalker != NULL && *ArgSetWalker != CHAR_NULL) {
> -  if (ArgSetWalker == NULL || *ArgSetWalker == CHAR_NULL || 
> !ShellIsValidForNumber(ArgSetWalker)){
> +if (EFI_ERROR(Status) || ArgSetWalker != NULL && 
> *ArgSetWalker != CHAR_NULL) {
> +  if (EFI_ERROR(Status) || ArgSetWalker == NULL || 
> *ArgSetWalker == CHAR_NULL || !ShellIsValidForNumber(ArgSetWalker)){
>  ShellPrintHiiEx(
>-1, 
>-1, 
> @@ -559,12 +541,13 @@ ShellCommandRu

[edk2] [PATCH] ShellPkg: Fix the wrong value got from 'for' command.

2015-07-14 Thread Qiu Shumin 
In 'For.c' return error status instead of a wrong number when fail to convert a 
string to UINT64.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jaben Carsey 
Reviewed-by: Qiu Shumin 
---
 ShellPkg/Library/UefiShellLevel1CommandsLib/For.c | 53 ---
 1 file changed, 18 insertions(+), 35 deletions(-)

diff --git a/ShellPkg/Library/UefiShellLevel1CommandsLib/For.c 
b/ShellPkg/Library/UefiShellLevel1CommandsLib/For.c
index 2ecc5cd..3a6852e 100644
--- a/ShellPkg/Library/UefiShellLevel1CommandsLib/For.c
+++ b/ShellPkg/Library/UefiShellLevel1CommandsLib/For.c
@@ -2,7 +2,7 @@
   Main file for endfor and for shell level 1 functions.
 
   (C) Copyright 2015 Hewlett-Packard Development Company, L.P.
-  Copyright (c) 2009 - 2014, Intel Corporation. All rights reserved.
+  Copyright (c) 2009 - 2015, Intel Corporation. All rights reserved.
   This program and the accompanying materials
   are licensed and made available under the terms and conditions of the BSD 
License
   which accompanies this distribution.  The full text of the license may be 
found at
@@ -255,29 +255,6 @@ InternalRemoveAliasFromList(
 }
 
 /**
-  Function to determine whether a string is decimal or hex representation of a 
number
-  and return the number converted from the string.
-
-  @param[in] String   String representation of a number
-
-  @return the number
-  @retval (UINTN)(-1) An error ocurred.
-**/
-UINTN
-EFIAPI
-ReturnUintn(
-  IN CONST CHAR16 *String
-  )
-{
-  UINT64RetVal;
-
-  if (!EFI_ERROR(ShellConvertStringToUint64(String, &RetVal, FALSE, TRUE))) {
-return ((UINTN)RetVal);
-  }
-  return ((UINTN)(-1));
-}
-
-/**
   Function for 'for' command.
 
   @param[in] ImageHandle  Handle to the Image (NULL if Internal).
@@ -305,6 +282,7 @@ ShellCommandRunFor (
   EFI_SHELL_FILE_INFO *Node;
   EFI_SHELL_FILE_INFO *FileList;
   UINTN   NewSize;
+  UINT64  Temp;
 
   ArgSet  = NULL;
   ArgSize = 0;
@@ -509,15 +487,17 @@ ShellCommandRunFor (
   ShellStatus = SHELL_INVALID_PARAMETER;
 } else {
   if (ArgSetWalker[0] == L'-') {
-Info->Current = 0 - (INTN)ReturnUintn(ArgSetWalker+1);
+Status = ShellConvertStringToUint64(ArgSetWalker+1, &Temp, 
FALSE, TRUE);
+Info->Current = 0 - (INTN)Temp;
   } else {
-Info->Current = (INTN)ReturnUintn(ArgSetWalker);
+Status = ShellConvertStringToUint64(ArgSetWalker, &Temp, 
FALSE, TRUE);
+Info->Current = (INTN)Temp;
   }
   ArgSetWalker  = StrStr(ArgSetWalker, L" ");
   while (ArgSetWalker != NULL && ArgSetWalker[0] == L' ') {
 ArgSetWalker++;
   }
-  if (ArgSetWalker == NULL || *ArgSetWalker == CHAR_NULL || 
!ShellIsValidForNumber(ArgSetWalker)){
+  if (EFI_ERROR(Status) || ArgSetWalker == NULL || *ArgSetWalker 
== CHAR_NULL || !ShellIsValidForNumber(ArgSetWalker)){
 ShellPrintHiiEx(
   -1, 
   -1, 
@@ -529,9 +509,11 @@ ShellCommandRunFor (
 ShellStatus = SHELL_INVALID_PARAMETER;
   } else {
 if (ArgSetWalker[0] == L'-') {
-  Info->End = 0 - (INTN)ReturnUintn(ArgSetWalker+1);
+  Status = ShellConvertStringToUint64(ArgSetWalker+1, &Temp, 
FALSE, TRUE);
+  Info->End = 0 - (INTN)Temp;
 } else {
-  Info->End = (INTN)ReturnUintn(ArgSetWalker);
+  Status = ShellConvertStringToUint64(ArgSetWalker, &Temp, 
FALSE, TRUE);
+  Info->End = (INTN)Temp;
 }
 if (Info->Current < Info->End) {
   Info->Step= 1;
@@ -543,8 +525,8 @@ ShellCommandRunFor (
 while (ArgSetWalker != NULL && ArgSetWalker[0] == L' ') {
   ArgSetWalker++;
 }
-if (ArgSetWalker != NULL && *ArgSetWalker != CHAR_NULL) {
-  if (ArgSetWalker == NULL || *ArgSetWalker == CHAR_NULL || 
!ShellIsValidForNumber(ArgSetWalker)){
+if (EFI_ERROR(Status) || ArgSetWalker != NULL && *ArgSetWalker 
!= CHAR_NULL) {
+  if (EFI_ERROR(Status) || ArgSetWalker == NULL || 
*ArgSetWalker == CHAR_NULL || !ShellIsValidForNumber(ArgSetWalker)){
 ShellPrintHiiEx(
   -1, 
   -1, 
@@ -559,12 +541,13 @@ ShellCommandRunFor (
   ASSERT(Info->Step == 1 || Info->Step == -1);
 } else {
   if (ArgSetWalker[0] == L'-') {
-Info->Step = 0 - (INTN)ReturnUintn(ArgSetWalker+1);
+Status = ShellConvertStringToUint64(ArgSetWalker+1, 
&Temp, FALSE, TRUE);
+Info->Step = 0 - (INTN)Temp;
   }