Re: [PATCH] ShellPkg/alias: Fix bug to support upper-case alias

["Ni, Ruiyu" <ruiyu.ni@intel.com>]

I was using AliasLower.
I am not sure whether the change is smallest. But I tried best to make the new implementation cleaner. I think that's what we really need.

Did you see any issue if we return EFI_NOT_FOUND (when variable doesn't exist)?

Regards,
Ray

>-----Original Message-----
>From: Carsey, Jaben
>Sent: Thursday, June 1, 2017 11:02 PM
>To: Ni, Ruiyu <ruiyu.ni@intel.com>; edk2-devel@lists.01.org
>Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Shah, Tapan (tapandshah@hpe.com) <tapandshah@hpe.com>
>Subject: RE: [PATCH] ShellPkg/alias: Fix bug to support upper-case alias
>
>Why not just use the AliasLower and make the overall change much smaller?  Looks like the old version did the conversion,
>but didn't use the result.
>
>Also, I notice that we are now checking the return value upon delete, which was explicitly not done in the old version.
>There was this comment before: "We dont check the error return on purpose since the variable may not exist."
>
>-Jaben
>
>
>> -----Original Message-----
>> From: Ni, Ruiyu
>> Sent: Thursday, June 01, 2017 7:12 AM
>> To: edk2-devel@lists.01.org
>> Cc: Carsey, Jaben <jaben.carsey@intel.com>; Kinney, Michael D
>> <michael.d.kinney@intel.com>
>> Subject: [PATCH] ShellPkg/alias: Fix bug to support upper-case alias
>> Importance: High
>>
>> alias in UEFI Shell is case insensitive.
>> Old code saves the alias to variable storage without
>> converting the alias to lower-case, which results
>> upper case alias setting doesn't work.
>> The patch converts the alias to lower case before saving
>> to variable storage.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
>> Cc: Jaben Carsey <jaben.carsey@intel.com>
>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>> ---
>>  ShellPkg/Application/Shell/ShellProtocol.c | 50 +++++++++++++++------------
>> ---
>>  1 file changed, 25 insertions(+), 25 deletions(-)
>>
>> diff --git a/ShellPkg/Application/Shell/ShellProtocol.c
>> b/ShellPkg/Application/Shell/ShellProtocol.c
>> index 347e162e62..b3b8acc0d0 100644
>> --- a/ShellPkg/Application/Shell/ShellProtocol.c
>> +++ b/ShellPkg/Application/Shell/ShellProtocol.c
>> @@ -3463,40 +3463,40 @@ InternalSetAlias(
>>  {
>>    EFI_STATUS  Status;
>>    CHAR16      *AliasLower;
>> +  BOOLEAN     DeleteAlias;
>>
>> -  // Convert to lowercase to make aliases case-insensitive
>> -  if (Alias != NULL) {
>> -    AliasLower = AllocateCopyPool (StrSize (Alias), Alias);
>> -    if (AliasLower == NULL) {
>> -      return EFI_OUT_OF_RESOURCES;
>> -    }
>> -    ToLower (AliasLower);
>> -  } else {
>> -    AliasLower = NULL;
>> -  }
>> -
>> -  //
>> -  // We must be trying to remove one if Alias is NULL
>> -  //
>> +  DeleteAlias = FALSE;
>>    if (Alias == NULL) {
>>      //
>> +    // We must be trying to remove one if Alias is NULL
>>      // remove an alias (but passed in COMMAND parameter)
>>      //
>> -    Status = (gRT->SetVariable((CHAR16*)Command, &gShellAliasGuid, 0, 0,
>> NULL));
>> -  } else {
>> -    //
>> -    // Add and replace are the same
>> -    //
>> -
>> -    // We dont check the error return on purpose since the variable may not
>> exist.
>> -    gRT->SetVariable((CHAR16*)Command, &gShellAliasGuid, 0, 0, NULL);
>> +    Alias       = Command;
>> +    DeleteAlias = TRUE;
>> +  }
>> +  ASSERT (Alias != NULL);
>>
>> -    Status = (gRT->SetVariable((CHAR16*)Alias, &gShellAliasGuid,
>> EFI_VARIABLE_BOOTSERVICE_ACCESS|(Volatile?0:EFI_VARIABLE_NON_VOL
>> ATILE), StrSize(Command), (VOID*)Command));
>> +  //
>> +  // Convert to lowercase to make aliases case-insensitive
>> +  //
>> +  AliasLower = AllocateCopyPool (StrSize (Alias), Alias);
>> +  if (AliasLower == NULL) {
>> +    return EFI_OUT_OF_RESOURCES;
>>    }
>> +  ToLower (AliasLower);
>>
>> -  if (Alias != NULL) {
>> -    FreePool (AliasLower);
>> +  if (DeleteAlias) {
>> +    Status = gRT->SetVariable (AliasLower, &gShellAliasGuid, 0, 0, NULL);
>> +  } else {
>> +    Status = gRT->SetVariable (
>> +                    AliasLower, &gShellAliasGuid,
>> +                    EFI_VARIABLE_BOOTSERVICE_ACCESS | (Volatile ? 0 :
>> EFI_VARIABLE_NON_VOLATILE),
>> +                    StrSize (Command), (VOID *) Command
>> +                    );
>>    }
>> +
>> +  FreePool (AliasLower);
>> +
>>    return Status;
>>  }
>>
>> --
>> 2.12.2.windows.2