From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from msmail.insydesw.com.tw (ms.insydesw.com [211.75.113.220]) (using TLSv1 with cipher AES128-SHA (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 59C2881DDF for ; Fri, 28 Oct 2016 09:48:48 -0700 (PDT) Received: from msmail.insydesw.com.tw ([fe80::74f7:f173:f4aa:9a05]) by msmail.insydesw.com.tw ([fe80::74f7:f173:f4aa:9a05%11]) with mapi id 14.01.0438.000; Sat, 29 Oct 2016 00:48:46 +0800 From: Tim Lewis To: "Carsey, Jaben" , "edk2-devel@lists.01.org" Thread-Topic: [shell] AliasLower never used in InternalSetAlias Thread-Index: AdIwlmPP8ooonpLXRf2RvwArhReYzQAAmqFgACVsmeAAAs2gQA== Date: Fri, 28 Oct 2016 16:48:45 +0000 Message-ID: <7236196A5DF6C040855A6D96F556A53F3FD93F@msmail.insydesw.com.tw> References: <7236196A5DF6C040855A6D96F556A53F3FD11D@msmail.insydesw.com.tw> <7236196A5DF6C040855A6D96F556A53F3FD173@msmail.insydesw.com.tw> In-Reply-To: Accept-Language: en-US, zh-TW X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [73.90.209.81] MIME-Version: 1.0 Subject: Re: [shell] AliasLower never used in InternalSetAlias X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 28 Oct 2016 16:48:48 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Jaben -- 1) I'm not talking about desired behavior, I'm talking about spec behavior.= As an external user of GetAlias() and SetAlias() (and a non-EDK2 shell imp= lementer), I have become aware of a number of places where the EDK2 shell m= akes assumptions. The spec requirement is that commands are case-insensiti= ve, but does not place this requirement on Alias. The way in which the comm= and-line uses aliases is not relevant to how the API responds. The fact is:= there might be entries for both Dir and dir, and the command-line usage in= this case is non-deterministic as the specification sits now. Obviously th= e spec needs an update to deal with what is currently a behavior which can = lead to unexpected API failures. 2) As I pointed out separately, the current code doesn't work as you (or I)= describe. It does not use AliasLower (the lower case version of the input = string) for the variable name in SetAlias, but does use AliasLower in GetAl= ias.=20 3) The specification also states that aliases are identifiers, but the inte= rnal usage for "cd.." and "cd\" do not follow this rule. Neither the comman= d or the API enforce this rule. BTW, another place I have found is in the mapping API, where the "cd" comma= nd currently relies on the fact that the 2nd parameter can have a mapping n= ame in it (while leaving the first parameter as a NULL). This is unexpected= behavior on the API part. Tim -----Original Message----- From: Carsey, Jaben [mailto:jaben.carsey@intel.com]=20 Sent: Friday, October 28, 2016 8:20 AM To: Tim Lewis ; edk2-devel@lists.01.org Cc: Carsey, Jaben Subject: RE: [shell] AliasLower never used in InternalSetAlias Tim, Given that all commands are case insensitive, I couldn't imagine why we wou= ld want case-sensitive alias. Do we really want "Dir" to fail, while "dir" works fine? Remember that the= real command is case insensitive "ls" in either case. -Jaben > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of=20 > Tim Lewis > Sent: Thursday, October 27, 2016 2:29 PM > To: Tim Lewis ; edk2-devel@lists.01.org > Subject: Re: [edk2] [shell] AliasLower never used in InternalSetAlias > Importance: High >=20 > I would also note that GetAlias() has similar logic, but does, in fact=20 > use the AliasLower. As far as I can tell, the specification does not=20 > say anything about case-insensitive, so I believe this to be in error. >=20 > Tim >=20 > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of=20 > Tim Lewis > Sent: Thursday, October 27, 2016 2:11 PM > To: edk2-devel@lists.01.org > Subject: [edk2] [shell] AliasLower never used in InternalSetAlias >=20 > In the function InternalSetAlias, it appears that AliasLower is=20 > duplicated (fromAlias), converted to lower case and freed ,but never=20 > actually used. Am I missing something? >=20 > // Convert to lowercase to make aliases case-insensitive > if (Alias !=3D NULL) { > AliasLower =3D AllocateCopyPool (StrSize (Alias), Alias); > if (AliasLower =3D=3D NULL) { > return EFI_OUT_OF_RESOURCES; > } > ToLower (AliasLower); > } else { > AliasLower =3D NULL; > } >=20 > // > // We must be trying to remove one if Alias is NULL > // > if (Alias =3D=3D NULL) { > // > // remove an alias (but passed in COMMAND parameter) > // > Status =3D (gRT->SetVariable((CHAR16*)Command, &gShellAliasGuid, 0,=20 > 0, NULL)); > } else { > // > // Add and replace are the same > // >=20 > // We dont check the error return on purpose since the variable=20 > may not exist. > gRT->SetVariable((CHAR16*)Command, &gShellAliasGuid, 0, 0, NULL); >=20 > Status =3D (gRT->SetVariable((CHAR16*)Alias, &gShellAliasGuid,=20 > EFI_VARIABLE_BOOTSERVICE_ACCESS|(Volatile?0:EFI_VARIABLE_NON_VOL > ATILE), StrSize(Command), (VOID*)Command)); > } >=20 > if (Alias !=3D NULL) { > FreePool (AliasLower); > } > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel