Re: [patch] ShellPkg/DP: Add more check for input parameters

["Carsey, Jaben" <jaben.carsey@intel.com>]

Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>

> -----Original Message-----
> From: Bi, Dandan
> Sent: Monday, June 25, 2018 10:13 PM
> To: edk2-devel@lists.01.org
> Cc: Gao, Liming <liming.gao@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>;
> Carsey, Jaben <jaben.carsey@intel.com>
> Subject: [patch] ShellPkg/DP: Add more check for input parameters
> Importance: High
> 
> New added checkers includes:
> 1. Too many invalid parameters
> 2. Too few parameter
> 3. Invalid number parameter for -n and -t flag
> 4. Conflict parameter of -A and -R.
> 
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Jaben Carsey <jaben.carsey@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Dandan Bi <dandan.bi@intel.com>
> ---
>  ShellPkg/DynamicCommand/DpDynamicCommand/Dp.c   | 105
> ++++++++++++++++--------
>  ShellPkg/DynamicCommand/DpDynamicCommand/Dp.uni |   6 ++
>  2 files changed, 77 insertions(+), 34 deletions(-)
> 
> diff --git a/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.c
> b/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.c
> index 38766613175..dea9ff85738 100644
> --- a/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.c
> +++ b/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.c
> @@ -718,10 +718,11 @@ RunDp (
>    CONST CHAR16              *CustomCumulativeToken;
>    PERF_CUM_DATA             *CustomCumulativeData;
>    UINTN                     NameSize;
>    SHELL_STATUS              ShellStatus;
>    TIMER_INFO                TimerInfo;
> +  UINT64                    Intermediate;
> 
>    StringPtr   = NULL;
>    SummaryMode = FALSE;
>    VerboseMode = FALSE;
>    AllMode     = FALSE;
> @@ -742,10 +743,13 @@ RunDp (
>    //
>    Status = ShellCommandLineParse (ParamList, &ParamPackage, NULL,
> TRUE);
>    if (EFI_ERROR(Status)) {
>      ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_DP_INVALID_ARG),
> mDpHiiHandle);
>      return SHELL_INVALID_PARAMETER;
> +  } else if (ShellCommandLineGetCount(ParamPackage) > 1){
> +    ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_DP_TOO_MANY),
> mDpHiiHandle);
> +    return SHELL_INVALID_PARAMETER;
>    }
> 
>    //
>    // Boolean options
>    //
> @@ -755,28 +759,84 @@ RunDp (
>    RawMode     = ShellCommandLineGetFlag (ParamPackage, L"-R");
>    ExcludeMode = ShellCommandLineGetFlag (ParamPackage, L"-x");
>    mShowId     = ShellCommandLineGetFlag (ParamPackage, L"-i");
>    CumulativeMode = ShellCommandLineGetFlag (ParamPackage, L"-c");
> 
> +  if (AllMode && RawMode) {
> +    ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_DP_CONFLICT_ARG),
> mDpHiiHandle, L"-A", L"-R");
> +    return SHELL_INVALID_PARAMETER;
> +  }
> +
>    // Options with Values
> -  CmdLineArg  = ShellCommandLineGetValue (ParamPackage, L"-n");
> -  if (CmdLineArg == NULL) {
> -    Number2Display = DEFAULT_DISPLAYCOUNT;
> -  } else {
> -    Number2Display = StrDecimalToUintn(CmdLineArg);
> -    if (Number2Display == 0) {
> -      Number2Display = MAXIMUM_DISPLAYCOUNT;
> +  if (ShellCommandLineGetFlag (ParamPackage, L"-n")) {
> +    CmdLineArg  = ShellCommandLineGetValue (ParamPackage, L"-n");
> +    if (CmdLineArg == NULL) {
> +      ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_DP_TOO_FEW),
> mDpHiiHandle);
> +      return SHELL_INVALID_PARAMETER;
> +    } else {
> +      if (!(RawMode || AllMode)) {
> +        ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_DP_NO_RAW_ALL),
> mDpHiiHandle);
> +        return SHELL_INVALID_PARAMETER;
> +      }
> +      Status = ShellConvertStringToUint64(CmdLineArg, &Intermediate, FALSE,
> TRUE);
> +      if (EFI_ERROR (Status)) {
> +        ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> (STR_DP_INVALID_NUM_ARG), mDpHiiHandle, L"-n");
> +        return SHELL_INVALID_PARAMETER;
> +      } else {
> +        Number2Display = (UINTN)Intermediate;
> +        if (Number2Display == 0 || Number2Display >
> MAXIMUM_DISPLAYCOUNT) {
> +          ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> (STR_DP_INVALID_RANGE), mDpHiiHandle, L"-n", 0,
> MAXIMUM_DISPLAYCOUNT);
> +          return SHELL_INVALID_PARAMETER;
> +        }
> +      }
>      }
> +  } else {
> +    Number2Display = DEFAULT_DISPLAYCOUNT;
>    }
> 
> -  CmdLineArg  = ShellCommandLineGetValue (ParamPackage, L"-t");
> -  if (CmdLineArg == NULL) {
> -    mInterestThreshold = DEFAULT_THRESHOLD;  // 1ms := 1,000 us
> +  if (ShellCommandLineGetFlag (ParamPackage, L"-t")) {
> +    CmdLineArg  = ShellCommandLineGetValue (ParamPackage, L"-t");
> +    if (CmdLineArg == NULL) {
> +      ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_DP_TOO_FEW),
> mDpHiiHandle);
> +      return SHELL_INVALID_PARAMETER;
> +    } else {
> +      Status = ShellConvertStringToUint64(CmdLineArg, &Intermediate, FALSE,
> TRUE);
> +      if (EFI_ERROR (Status)) {
> +        ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> (STR_DP_INVALID_NUM_ARG), mDpHiiHandle, L"-t");
> +        return SHELL_INVALID_PARAMETER;
> +      } else {
> +        mInterestThreshold = Intermediate;
> +      }
> +    }
>    } else {
> -    mInterestThreshold = StrDecimalToUint64(CmdLineArg);
> +    mInterestThreshold = DEFAULT_THRESHOLD;  // 1ms := 1,000 us
>    }
> 
> +  if (ShellCommandLineGetFlag (ParamPackage, L"-c")) {
> +    CustomCumulativeToken = ShellCommandLineGetValue (ParamPackage,
> L"-c");
> +    if (CustomCumulativeToken == NULL) {
> +      ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_DP_TOO_FEW),
> mDpHiiHandle);
> +      return SHELL_INVALID_PARAMETER;
> +    } else {
> +      CustomCumulativeData = AllocateZeroPool (sizeof (PERF_CUM_DATA));
> +      if (CustomCumulativeData == NULL) {
> +        ShellStatus = SHELL_OUT_OF_RESOURCES;
> +        goto Done;
> +      }
> +      CustomCumulativeData->MinDur = PERF_MAXDUR;
> +      CustomCumulativeData->MaxDur = 0;
> +      CustomCumulativeData->Count  = 0;
> +      CustomCumulativeData->Duration = 0;
> +      NameSize = StrLen (CustomCumulativeToken) + 1;
> +      CustomCumulativeData->Name   = AllocateZeroPool (NameSize);
> +      if (CustomCumulativeData->Name == NULL) {
> +        ShellStatus = SHELL_OUT_OF_RESOURCES;
> +        goto Done;
> +      }
> +      UnicodeStrToAsciiStrS (CustomCumulativeToken,
> CustomCumulativeData->Name, NameSize);
> +    }
> +  }
> 
>    //
>    // DP dump performance data by parsing FPDT table in ACPI table.
>    // Folloing 3 steps are to get the measurement form the FPDT table.
>    //
> @@ -816,33 +876,10 @@ RunDp (
>    //
>    // Initialize the Summary data.
>    //
>    InitSummaryData ();
> 
> -  //
> -  // Init the custom cumulative data.
> -  //
> -  CustomCumulativeToken = ShellCommandLineGetValue (ParamPackage,
> L"-c");
> -  if (CustomCumulativeToken != NULL) {
> -    CustomCumulativeData = AllocateZeroPool (sizeof (PERF_CUM_DATA));
> -    if (CustomCumulativeData == NULL) {
> -      ShellStatus = SHELL_OUT_OF_RESOURCES;
> -      goto Done;
> -    }
> -    CustomCumulativeData->MinDur = PERF_MAXDUR;
> -    CustomCumulativeData->MaxDur = 0;
> -    CustomCumulativeData->Count  = 0;
> -    CustomCumulativeData->Duration = 0;
> -    NameSize = StrLen (CustomCumulativeToken) + 1;
> -    CustomCumulativeData->Name   = AllocateZeroPool (NameSize);
> -    if (CustomCumulativeData->Name == NULL) {
> -      ShellStatus = SHELL_OUT_OF_RESOURCES;
> -      goto Done;
> -    }
> -    UnicodeStrToAsciiStrS (CustomCumulativeToken, CustomCumulativeData-
> >Name, NameSize);
> -  }
> -
>    //
>    // Timer specific processing
>    //
>    // Get the Performance counter characteristics:
>    //          Freq = Frequency in Hz
> diff --git a/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.uni
> b/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.uni
> index ced8a487428..bde499fb873 100644
> --- a/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.uni
> +++ b/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.uni
> @@ -31,10 +31,16 @@
>  #string STR_DP_UP                      #language en-US  "UP"
>  #string STR_DP_DOWN                    #language en-US  "DOWN"
>  #string STR_DP_DASHES                  #language en-US  "-----------------------------
> --------------------------------------------------\n"
>  #string STR_DP_SECTION_HEADER          #language en-US  "\n==[ %s
> ]========\n"
>  #string STR_DP_INVALID_ARG             #language en-US  "Invalid
> argument(s)\n"
> +#string STR_DP_TOO_MANY                #language en-US  "Too many
> arguments\n"
> +#string STR_DP_TOO_FEW                 #language en-US  "Too few
> arguments\n"
> +#string STR_DP_INVALID_NUM_ARG         #language en-US  "Invalid
> argument(s), the value of %H%s%N must be numbers\n"
> +#string STR_DP_INVALID_RANGE           #language en-US  "Invalid
> argument(s), the value of %H%s%N must be between %H%d%N and
> %H%d%N\n"
> +#string STR_DP_CONFLICT_ARG            #language en-US  "Invalid
> argument(s), %H%s%N can not be used together with %H%s%N\n"
> +#string STR_DP_NO_RAW_ALL              #language en-US  "Invalid
> argument(s), -n flag must use with -A or -R\n"
>  #string STR_DP_HANDLES_ERROR           #language en-US  "Locate all handles
> error - %r\n"
>  #string STR_DP_ERROR_NAME              #language en-US  "Unknown driver
> name"
>  #string STR_PERF_PROPERTY_NOT_FOUND    #language en-US
> "Performance property not found\n"
>  #string STR_DP_BUILD_REVISION          #language en-US  "\nDP Build
> Version:       %d.%d\n"
>  #string STR_DP_KHZ                     #language en-US  "System Performance
> Timer Frequency:   %,8d (KHz)\n"
> --
> 2.14.3.windows.1