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 <[email protected]>; [email protected] >Cc: Kinney, Michael D <[email protected]>; Shah, Tapan >([email protected]) <[email protected]> >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: [email protected] >> Cc: Carsey, Jaben <[email protected]>; Kinney, Michael D >> <[email protected]> >> 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 <[email protected]> >> Cc: Jaben Carsey <[email protected]> >> Cc: Michael D Kinney <[email protected]> >> --- >> 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 _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
