On 03/31/15 20:14, Jordan Justen wrote: > In 1/2, it seems like the typo came from the Shell spec 2.0, but it > was fixed in 2.1. > > My one minor comment is that in 2/2 it could have been > IsStringOfHexBytes, and trivially verified an even number of > characters without the extra StrLen call later on.
You're absolutely right. I definitely saw the room for improvement, but not just in my patch: in the surrounding code (in SetVar.c) as well. And that's when I depend on the maintainer to guide me -- I can start refactoring, but where do I stop? So, whenever I touch a file or subsystem for the very first time, I try to be as surgical as possible. I didn't want to change any roles or logic here, unless told so in review. You did propose an improvement, however: > Series Reviewed-by: Jordan Justen <[email protected]> you were also okay with this version (probably because it didn't regress anything, it just didn't improve things as far as it could have). So, I pushed this series as SVN r17127-r17128. (I didn't wait for Jaben's review any longer... I posted the series ~10 days ago, and it was a simple one.) Thanks! Laszlo > > On 2015-03-27 17:23:04, Laszlo Ersek wrote: >> Cc: Jaben Carsey <[email protected]> >> >> Sometimes it is useful to set UEFI variables via machine-generated UEFI >> shell scripts, with the help of the SETVAR command. The hexadecimal data >> string parameter of SETVAR is ideal for this purpose -- the script >> generator can easily prepare such arguments --, however it turns out >> that the hex data string parser of SETVAR has an unintended length >> limitation. Let's lift it. >> >> (If the patches are deemed correct, I'd prefer to commit them myself to >> SVN.) >> >> Thanks! >> Laszlo >> >> Laszlo Ersek (2): >> ShellPkg: UefiShellDebug1CommandsLib: fix SETVAR option summary >> ShellPkg: UefiShellDebug1CommandsLib: fix hex string parsing in SETVAR >> >> ShellPkg/Library/UefiShellDebug1CommandsLib/SetVar.c >> | 32 +++++++++++++++++++- >> ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.uni >> | Bin 140130 -> 140130 bytes >> 2 files changed, 31 insertions(+), 1 deletion(-) >> >> -- >> 1.8.3.1 >> >> >> ------------------------------------------------------------------------------ >> Dive into the World of Parallel Programming The Go Parallel Website, >> sponsored >> by Intel and developed in partnership with Slashdot Media, is your hub for >> all >> things parallel software development, from weekly thought leadership blogs to >> news, videos, case studies, tutorials and more. Take a look and join the >> conversation now. http://goparallel.sourceforge.net/ >> _______________________________________________ >> edk2-devel mailing list >> [email protected] >> https://lists.sourceforge.net/lists/listinfo/edk2-devel ------------------------------------------------------------------------------ BPM Camp - Free Virtual Workshop May 6th at 10am PDT/1PM EDT Develop your own process in accordance with the BPMN 2 standard Learn Process modeling best practices with Bonita BPM through live exercises http://www.bonitasoft.com/be-part-of-it/events/bpm-camp-virtual- event?utm_ source=Sourceforge_BPM_Camp_5_6_15&utm_medium=email&utm_campaign=VA_SF _______________________________________________ edk2-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/edk2-devel
