Re: [edk2-devel] [PATCH v1 1/1] ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' option
I see this landed last week. Many thanks to everyone for their input and for getting this landed. Jonathan On 05/08/2019 01:51, Gao, Zhichao wrote: > Agree. I would prepare this patch for push. > > Thanks, > Zhichao > >> -Original Message- >> From: Carsey, Jaben >> Sent: Saturday, August 3, 2019 5:24 AM >> To: devel@edk2.groups.io; jw...@jwatt.org >> Cc: tim.le...@insyde.com; Gao, Zhichao ; Ni, Ray >> ; Bi, Dandan ; Rothman, Michael >> A >> Subject: RE: [edk2-devel] [PATCH v1 1/1] ShellPkg/UefiShellBcfgCommandLib: >> Fix '-opt' option >> >> I think we can push this in now. >> >> Zhichao, >> Do you agree? If yes, can you prep this for merging? >> >> Thanks >> -Jaben >> -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#45465): https://edk2.groups.io/g/devel/message/45465 Mute This Topic: https://groups.io/mt/31520134/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v1 1/1] ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' option
Agree. I would prepare this patch for push. Thanks, Zhichao > -Original Message- > From: Carsey, Jaben > Sent: Saturday, August 3, 2019 5:24 AM > To: devel@edk2.groups.io; jw...@jwatt.org > Cc: tim.le...@insyde.com; Gao, Zhichao ; Ni, Ray > ; Bi, Dandan ; Rothman, Michael > A > Subject: RE: [edk2-devel] [PATCH v1 1/1] ShellPkg/UefiShellBcfgCommandLib: > Fix '-opt' option > > I think we can push this in now. > > Zhichao, > Do you agree? If yes, can you prep this for merging? > > Thanks > -Jaben > > > -Original Message- > > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of > > Jonathan Watt > > Sent: Friday, August 02, 2019 1:28 PM > > To: devel@edk2.groups.io > > Cc: tim.le...@insyde.com; Carsey, Jaben ; Gao, > > Zhichao ; Ni, Ray ; Bi, > > Dandan > > Subject: Re: [edk2-devel] [PATCH v1 1/1] > > ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' option > > > > It's been three months now since I contributed the patch. Could > > someone update me on the progress on getting it landed? > > > > On 11/06/2019 22:53, Jonathan Watt wrote: > > > Since I haven't contributed before I'm not sure what the timeline > > > for these things generally is. It's been a month though. Can the > > > patch be pushed > > now? > > > > > > Regards, > > > Jonathan > > > > > > On 08/05/2019 01:08, Tim Lewis wrote: > > >> Yes, I would support it. Tim > > >> > > >> -Original Message- > > >> From: Carsey, Jaben > > >> Sent: Tuesday, May 7, 2019 5:00 PM > > >> To: Jonathan Watt ; devel@edk2.groups.io; > > tim.le...@insyde.com; Gao, Zhichao ; Ni, Ray > > > > >> Cc: Bi, Dandan > > >> Subject: RE: [edk2-devel] [PATCH v1 1/1] > > ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' option > > >> > > >> Tim, > > >> > > >> Does this mean you would support such an errata? I would like to > > >> get the > > spec to a place where the behavior is at least nailed down one way or > > the other... > > >> > > >> -Jaben > > >> > > >>> -Original Message- > > >>> From: Jonathan Watt [mailto:jw...@jwatt.org] > > >>> Sent: Tuesday, May 7, 2019 2:08 PM > > >>> To: devel@edk2.groups.io; tim.le...@insyde.com; Carsey, Jaben > > >>> ; Gao, Zhichao ; > > >>> Ni, Ray > > >>> Cc: Bi, Dandan > > >>> Subject: Re: [edk2-devel] [PATCH v1 1/1] > > ShellPkg/UefiShellBcfgCommandLib: > > >>> Fix '-opt' option > > >>> Importance: High > > >>> > > >>> No apologies necessary! Raising compatibility concerns is very valid. > > >>> As I said, I just wanted to provide some other considerations I > > >>> saw to weigh in the decision. > > >>> > > >>> All the best, > > >>> Jonathan > > >>> > > >>> On 07/05/2019 22:02, Tim Lewis wrote: > > >>>> Jonathan -- > > >>>> > > >>>> My apologies. I jumped because we've been bitten by shell > > "clarifications" > > >>> in the past. > > >>>> > > >>>> As you've probably read in the other thread, it turns out that I > > >>>> (we) actually > > >>> did agree with your interpretation of the spec in our alternate > > >>> implementation and have been using it that way for 2+ years. And > > >>> it didn't cause us grief with our other product which does use an > > >>> EDK2- > > derived shell. > > >>>> > > >>>> Best regards, > > >>>> Tim > > >>>> > > >>>> -Original Message- > > >>>> From: devel@edk2.groups.io On Behalf Of > > >>>> Jonathan Watt > > >>>> Sent: Tuesday, May 7, 2019 1:51 PM > > >>>> To: Tim Lewis ; 'Carsey, Jaben' > > >>>> ; devel@edk2.groups.io; 'Gao, Zhichao' > > >>>> ; 'Ni, Ray' > > >>>> Cc: 'Bi, Dandan' > > >>>> Subject: Re: [edk2-devel] [PATCH v1 1/1] > > >>>> ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' option > > >>>> > > >>>> Hi Tim, > > >>>> > > >>>> For context, I'm just some random guy who tripped over this issue > > >
Re: [edk2-devel] [PATCH v1 1/1] ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' option
I think we can push this in now. Zhichao, Do you agree? If yes, can you prep this for merging? Thanks -Jaben > -Original Message- > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of > Jonathan Watt > Sent: Friday, August 02, 2019 1:28 PM > To: devel@edk2.groups.io > Cc: tim.le...@insyde.com; Carsey, Jaben ; Gao, > Zhichao ; Ni, Ray ; Bi, Dandan > > Subject: Re: [edk2-devel] [PATCH v1 1/1] > ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' option > > It's been three months now since I contributed the patch. Could someone > update > me on the progress on getting it landed? > > On 11/06/2019 22:53, Jonathan Watt wrote: > > Since I haven't contributed before I'm not sure what the timeline for these > > things generally is. It's been a month though. Can the patch be pushed > now? > > > > Regards, > > Jonathan > > > > On 08/05/2019 01:08, Tim Lewis wrote: > >> Yes, I would support it. Tim > >> > >> -Original Message- > >> From: Carsey, Jaben > >> Sent: Tuesday, May 7, 2019 5:00 PM > >> To: Jonathan Watt ; devel@edk2.groups.io; > tim.le...@insyde.com; Gao, Zhichao ; Ni, Ray > > >> Cc: Bi, Dandan > >> Subject: RE: [edk2-devel] [PATCH v1 1/1] > ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' option > >> > >> Tim, > >> > >> Does this mean you would support such an errata? I would like to get the > spec to a place where the behavior is at least nailed down one way or the > other... > >> > >> -Jaben > >> > >>> -Original Message----- > >>> From: Jonathan Watt [mailto:jw...@jwatt.org] > >>> Sent: Tuesday, May 7, 2019 2:08 PM > >>> To: devel@edk2.groups.io; tim.le...@insyde.com; Carsey, Jaben > >>> ; Gao, Zhichao ; Ni, > >>> Ray > >>> Cc: Bi, Dandan > >>> Subject: Re: [edk2-devel] [PATCH v1 1/1] > ShellPkg/UefiShellBcfgCommandLib: > >>> Fix '-opt' option > >>> Importance: High > >>> > >>> No apologies necessary! Raising compatibility concerns is very valid. > >>> As I said, I just wanted to provide some other considerations I saw to > >>> weigh in the decision. > >>> > >>> All the best, > >>> Jonathan > >>> > >>> On 07/05/2019 22:02, Tim Lewis wrote: > >>>> Jonathan -- > >>>> > >>>> My apologies. I jumped because we've been bitten by shell > "clarifications" > >>> in the past. > >>>> > >>>> As you've probably read in the other thread, it turns out that I > >>>> (we) actually > >>> did agree with your interpretation of the spec in our alternate > >>> implementation and have been using it that way for 2+ years. And it > >>> didn't cause us grief with our other product which does use an EDK2- > derived shell. > >>>> > >>>> Best regards, > >>>> Tim > >>>> > >>>> -Original Message- > >>>> From: devel@edk2.groups.io On Behalf Of > >>>> Jonathan Watt > >>>> Sent: Tuesday, May 7, 2019 1:51 PM > >>>> To: Tim Lewis ; 'Carsey, Jaben' > >>>> ; devel@edk2.groups.io; 'Gao, Zhichao' > >>>> ; 'Ni, Ray' > >>>> Cc: 'Bi, Dandan' > >>>> Subject: Re: [edk2-devel] [PATCH v1 1/1] > >>>> ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' option > >>>> > >>>> Hi Tim, > >>>> > >>>> For context, I'm just some random guy who tripped over this issue on > >>>> his > >>> home workstation and thought he'd try and remove the footgun to save > >>> anyone else the same pain. I was specifically replying to the > >>> unconditional statement "It will break existing scripts." (not made by > >>> you) to provide what I hope was some qualification and balance to the > >>> face value of that statement, and to suggest some other things that > >>> should be considered. As far as deciding what the best resolution is, I'm > not qualified for that. > >>>> > >>>> I am curious about one thing though. The sentence you wrote that > >>>> ends > >>> with "that are implemented to the specification" sounds like you're > >>> saying making the proposed change would violate the specification. > >>> That does not seem to be the case from my reading, and my readi
Re: [edk2-devel] [PATCH v1 1/1] ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' option
It's been three months now since I contributed the patch. Could someone update me on the progress on getting it landed? On 11/06/2019 22:53, Jonathan Watt wrote: > Since I haven't contributed before I'm not sure what the timeline for these > things generally is. It's been a month though. Can the patch be pushed now? > > Regards, > Jonathan > > On 08/05/2019 01:08, Tim Lewis wrote: >> Yes, I would support it. Tim >> >> -Original Message- >> From: Carsey, Jaben >> Sent: Tuesday, May 7, 2019 5:00 PM >> To: Jonathan Watt ; devel@edk2.groups.io; >> tim.le...@insyde.com; Gao, Zhichao ; Ni, Ray >> >> Cc: Bi, Dandan >> Subject: RE: [edk2-devel] [PATCH v1 1/1] ShellPkg/UefiShellBcfgCommandLib: >> Fix '-opt' option >> >> Tim, >> >> Does this mean you would support such an errata? I would like to get the >> spec to a place where the behavior is at least nailed down one way or the >> other... >> >> -Jaben >> >>> -Original Message- >>> From: Jonathan Watt [mailto:jw...@jwatt.org] >>> Sent: Tuesday, May 7, 2019 2:08 PM >>> To: devel@edk2.groups.io; tim.le...@insyde.com; Carsey, Jaben >>> ; Gao, Zhichao ; Ni, >>> Ray >>> Cc: Bi, Dandan >>> Subject: Re: [edk2-devel] [PATCH v1 1/1] ShellPkg/UefiShellBcfgCommandLib: >>> Fix '-opt' option >>> Importance: High >>> >>> No apologies necessary! Raising compatibility concerns is very valid. >>> As I said, I just wanted to provide some other considerations I saw to >>> weigh in the decision. >>> >>> All the best, >>> Jonathan >>> >>> On 07/05/2019 22:02, Tim Lewis wrote: >>>> Jonathan -- >>>> >>>> My apologies. I jumped because we've been bitten by shell "clarifications" >>> in the past. >>>> >>>> As you've probably read in the other thread, it turns out that I >>>> (we) actually >>> did agree with your interpretation of the spec in our alternate >>> implementation and have been using it that way for 2+ years. And it >>> didn't cause us grief with our other product which does use an EDK2-derived >>> shell. >>>> >>>> Best regards, >>>> Tim >>>> >>>> -Original Message- >>>> From: devel@edk2.groups.io On Behalf Of >>>> Jonathan Watt >>>> Sent: Tuesday, May 7, 2019 1:51 PM >>>> To: Tim Lewis ; 'Carsey, Jaben' >>>> ; devel@edk2.groups.io; 'Gao, Zhichao' >>>> ; 'Ni, Ray' >>>> Cc: 'Bi, Dandan' >>>> Subject: Re: [edk2-devel] [PATCH v1 1/1] >>>> ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' option >>>> >>>> Hi Tim, >>>> >>>> For context, I'm just some random guy who tripped over this issue on >>>> his >>> home workstation and thought he'd try and remove the footgun to save >>> anyone else the same pain. I was specifically replying to the >>> unconditional statement "It will break existing scripts." (not made by >>> you) to provide what I hope was some qualification and balance to the >>> face value of that statement, and to suggest some other things that >>> should be considered. As far as deciding what the best resolution is, I'm >>> not qualified for that. >>>> >>>> I am curious about one thing though. The sentence you wrote that >>>> ends >>> with "that are implemented to the specification" sounds like you're >>> saying making the proposed change would violate the specification. >>> That does not seem to be the case from my reading, and my reading >>> would be that it would actually make it do what most people would >>> expect from reading the specification. >>>> >>>> Specifically, the usage block for bcfg in the specification says: >>>> >>>> Usage: >>>> bcfg driver|boot [dump [-v]] >>>> bcfg driver|boot [add # file "desc"] [addp # file “desc”] >>>> [addh # handle “desc”] >>>> bcfg driver|boot [rm #] >>>> bcfg driver|boot [mv # #] >>>> bcfg driver|boot [mod # “desc”] | [modf # file] | [modp # file] | >>>> [modh # handle] >>>> bcfg driver|boot [-opt # [[filename]|[”data”]] | >>>> [KeyData *]] >>>> >>>> It seems natural to assu
Re: [edk2-devel] [PATCH v1 1/1] ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' option
Since I haven't contributed before I'm not sure what the timeline for these things generally is. It's been a month though. Can the patch be pushed now? Regards, Jonathan On 08/05/2019 01:08, Tim Lewis wrote: > Yes, I would support it. Tim > > -Original Message- > From: Carsey, Jaben > Sent: Tuesday, May 7, 2019 5:00 PM > To: Jonathan Watt ; devel@edk2.groups.io; > tim.le...@insyde.com; Gao, Zhichao ; Ni, Ray > > Cc: Bi, Dandan > Subject: RE: [edk2-devel] [PATCH v1 1/1] ShellPkg/UefiShellBcfgCommandLib: > Fix '-opt' option > > Tim, > > Does this mean you would support such an errata? I would like to get the spec > to a place where the behavior is at least nailed down one way or the other... > > -Jaben > >> -Original Message- >> From: Jonathan Watt [mailto:jw...@jwatt.org] >> Sent: Tuesday, May 7, 2019 2:08 PM >> To: devel@edk2.groups.io; tim.le...@insyde.com; Carsey, Jaben >> ; Gao, Zhichao ; Ni, >> Ray >> Cc: Bi, Dandan >> Subject: Re: [edk2-devel] [PATCH v1 1/1] ShellPkg/UefiShellBcfgCommandLib: >> Fix '-opt' option >> Importance: High >> >> No apologies necessary! Raising compatibility concerns is very valid. >> As I said, I just wanted to provide some other considerations I saw to >> weigh in the decision. >> >> All the best, >> Jonathan >> >> On 07/05/2019 22:02, Tim Lewis wrote: >>> Jonathan -- >>> >>> My apologies. I jumped because we've been bitten by shell "clarifications" >> in the past. >>> >>> As you've probably read in the other thread, it turns out that I >>> (we) actually >> did agree with your interpretation of the spec in our alternate >> implementation and have been using it that way for 2+ years. And it >> didn't cause us grief with our other product which does use an EDK2-derived >> shell. >>> >>> Best regards, >>> Tim >>> >>> -Original Message----- >>> From: devel@edk2.groups.io On Behalf Of >>> Jonathan Watt >>> Sent: Tuesday, May 7, 2019 1:51 PM >>> To: Tim Lewis ; 'Carsey, Jaben' >>> ; devel@edk2.groups.io; 'Gao, Zhichao' >>> ; 'Ni, Ray' >>> Cc: 'Bi, Dandan' >>> Subject: Re: [edk2-devel] [PATCH v1 1/1] >>> ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' option >>> >>> Hi Tim, >>> >>> For context, I'm just some random guy who tripped over this issue on >>> his >> home workstation and thought he'd try and remove the footgun to save >> anyone else the same pain. I was specifically replying to the >> unconditional statement "It will break existing scripts." (not made by >> you) to provide what I hope was some qualification and balance to the >> face value of that statement, and to suggest some other things that >> should be considered. As far as deciding what the best resolution is, I'm >> not qualified for that. >>> >>> I am curious about one thing though. The sentence you wrote that >>> ends >> with "that are implemented to the specification" sounds like you're >> saying making the proposed change would violate the specification. >> That does not seem to be the case from my reading, and my reading >> would be that it would actually make it do what most people would >> expect from reading the specification. >>> >>> Specifically, the usage block for bcfg in the specification says: >>> >>> Usage: >>> bcfg driver|boot [dump [-v]] >>> bcfg driver|boot [add # file "desc"] [addp # file “desc”] >>> [addh # handle “desc”] >>> bcfg driver|boot [rm #] >>> bcfg driver|boot [mv # #] >>> bcfg driver|boot [mod # “desc”] | [modf # file] | [modp # file] | >>> [modh # handle] >>> bcfg driver|boot [-opt # [[filename]|[”data”]] | >>> [KeyData *]] >>> >>> It seems natural to assume from that that the "#" for all options is >>> the >> "same thing" and would be handled the same way. >>> >>> The comment for the -opt option does not indicate otherwise: >>> >>> -opt >>> Modify the optional data associated with a driver or boot option. >>> Followed either by the filename of the file which contains the >>> binary data to be associated with the driver or boot option >>> optional data, or else the quote-delimited data that will be >>> assoc
Re: [edk2-devel] [PATCH v1 1/1] ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' option
Yes, I would support it. Tim -Original Message- From: Carsey, Jaben Sent: Tuesday, May 7, 2019 5:00 PM To: Jonathan Watt ; devel@edk2.groups.io; tim.le...@insyde.com; Gao, Zhichao ; Ni, Ray Cc: Bi, Dandan Subject: RE: [edk2-devel] [PATCH v1 1/1] ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' option Tim, Does this mean you would support such an errata? I would like to get the spec to a place where the behavior is at least nailed down one way or the other... -Jaben > -Original Message- > From: Jonathan Watt [mailto:jw...@jwatt.org] > Sent: Tuesday, May 7, 2019 2:08 PM > To: devel@edk2.groups.io; tim.le...@insyde.com; Carsey, Jaben > ; Gao, Zhichao ; Ni, > Ray > Cc: Bi, Dandan > Subject: Re: [edk2-devel] [PATCH v1 1/1] ShellPkg/UefiShellBcfgCommandLib: > Fix '-opt' option > Importance: High > > No apologies necessary! Raising compatibility concerns is very valid. > As I said, I just wanted to provide some other considerations I saw to > weigh in the decision. > > All the best, > Jonathan > > On 07/05/2019 22:02, Tim Lewis wrote: > > Jonathan -- > > > > My apologies. I jumped because we've been bitten by shell "clarifications" > in the past. > > > > As you've probably read in the other thread, it turns out that I > > (we) actually > did agree with your interpretation of the spec in our alternate > implementation and have been using it that way for 2+ years. And it > didn't cause us grief with our other product which does use an EDK2-derived > shell. > > > > Best regards, > > Tim > > > > -Original Message- > > From: devel@edk2.groups.io On Behalf Of > > Jonathan Watt > > Sent: Tuesday, May 7, 2019 1:51 PM > > To: Tim Lewis ; 'Carsey, Jaben' > > ; devel@edk2.groups.io; 'Gao, Zhichao' > > ; 'Ni, Ray' > > Cc: 'Bi, Dandan' > > Subject: Re: [edk2-devel] [PATCH v1 1/1] > > ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' option > > > > Hi Tim, > > > > For context, I'm just some random guy who tripped over this issue on > > his > home workstation and thought he'd try and remove the footgun to save > anyone else the same pain. I was specifically replying to the > unconditional statement "It will break existing scripts." (not made by > you) to provide what I hope was some qualification and balance to the > face value of that statement, and to suggest some other things that > should be considered. As far as deciding what the best resolution is, I'm not > qualified for that. > > > > I am curious about one thing though. The sentence you wrote that > > ends > with "that are implemented to the specification" sounds like you're > saying making the proposed change would violate the specification. > That does not seem to be the case from my reading, and my reading > would be that it would actually make it do what most people would > expect from reading the specification. > > > > Specifically, the usage block for bcfg in the specification says: > > > > Usage: > > bcfg driver|boot [dump [-v]] > > bcfg driver|boot [add # file "desc"] [addp # file “desc”] > > [addh # handle “desc”] > > bcfg driver|boot [rm #] > > bcfg driver|boot [mv # #] > > bcfg driver|boot [mod # “desc”] | [modf # file] | [modp # file] | > > [modh # handle] > > bcfg driver|boot [-opt # [[filename]|[”data”]] | > > [KeyData *]] > > > > It seems natural to assume from that that the "#" for all options is > > the > "same thing" and would be handled the same way. > > > > The comment for the -opt option does not indicate otherwise: > > > > -opt > > Modify the optional data associated with a driver or boot option. > > Followed either by the filename of the file which contains the > > binary data to be associated with the driver or boot option > > optional data, or else the quote-delimited data that will be > > associated with the driver or boot option optional data. > > > > In fact the use of the term "driver or boot option" for -opt and the > > other > options indicates that it is the same thing as for the other options > (which explicitly say that the "#" is a hexadecimal number), even if > "#" isn't described explicitly in this case. > > > > I'm glad to hear there are other implementations, because given the > disagreement over what the spec intends, it would be useful to compare > them and consider converging. > > >
Re: [edk2-devel] [PATCH v1 1/1] ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' option
Tim, Does this mean you would support such an errata? I would like to get the spec to a place where the behavior is at least nailed down one way or the other... -Jaben > -Original Message- > From: Jonathan Watt [mailto:jw...@jwatt.org] > Sent: Tuesday, May 7, 2019 2:08 PM > To: devel@edk2.groups.io; tim.le...@insyde.com; Carsey, Jaben > ; Gao, Zhichao ; Ni, Ray > > Cc: Bi, Dandan > Subject: Re: [edk2-devel] [PATCH v1 1/1] ShellPkg/UefiShellBcfgCommandLib: > Fix '-opt' option > Importance: High > > No apologies necessary! Raising compatibility concerns is very valid. As I > said, > I just wanted to provide some other considerations I saw to weigh in the > decision. > > All the best, > Jonathan > > On 07/05/2019 22:02, Tim Lewis wrote: > > Jonathan -- > > > > My apologies. I jumped because we've been bitten by shell "clarifications" > in the past. > > > > As you've probably read in the other thread, it turns out that I (we) > > actually > did agree with your interpretation of the spec in our alternate > implementation and have been using it that way for 2+ years. And it didn't > cause us grief with our other product which does use an EDK2-derived shell. > > > > Best regards, > > Tim > > > > -Original Message- > > From: devel@edk2.groups.io On Behalf Of > > Jonathan Watt > > Sent: Tuesday, May 7, 2019 1:51 PM > > To: Tim Lewis ; 'Carsey, Jaben' > > ; devel@edk2.groups.io; 'Gao, Zhichao' > > ; 'Ni, Ray' > > Cc: 'Bi, Dandan' > > Subject: Re: [edk2-devel] [PATCH v1 1/1] > > ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' option > > > > Hi Tim, > > > > For context, I'm just some random guy who tripped over this issue on his > home workstation and thought he'd try and remove the footgun to save > anyone else the same pain. I was specifically replying to the unconditional > statement "It will break existing scripts." (not made by you) to provide what > I > hope was some qualification and balance to the face value of that statement, > and to suggest some other things that should be considered. As far as > deciding what the best resolution is, I'm not qualified for that. > > > > I am curious about one thing though. The sentence you wrote that ends > with "that are implemented to the specification" sounds like you're saying > making the proposed change would violate the specification. That does not > seem to be the case from my reading, and my reading would be that it would > actually make it do what most people would expect from reading the > specification. > > > > Specifically, the usage block for bcfg in the specification says: > > > > Usage: > > bcfg driver|boot [dump [-v]] > > bcfg driver|boot [add # file "desc"] [addp # file “desc”] > > [addh # handle “desc”] > > bcfg driver|boot [rm #] > > bcfg driver|boot [mv # #] > > bcfg driver|boot [mod # “desc”] | [modf # file] | [modp # file] | > > [modh # handle] > > bcfg driver|boot [-opt # [[filename]|[”data”]] | > > [KeyData *]] > > > > It seems natural to assume from that that the "#" for all options is the > "same thing" and would be handled the same way. > > > > The comment for the -opt option does not indicate otherwise: > > > > -opt > > Modify the optional data associated with a driver or boot option. > > Followed either by the filename of the file which contains the > > binary data to be associated with the driver or boot option > > optional data, or else the quote-delimited data that will be > > associated with the driver or boot option optional data. > > > > In fact the use of the term "driver or boot option" for -opt and the other > options indicates that it is the same thing as for the other options (which > explicitly say that the "#" is a hexadecimal number), even if "#" isn't > described explicitly in this case. > > > > I'm glad to hear there are other implementations, because given the > disagreement over what the spec intends, it would be useful to compare > them and consider converging. > > > > Anyway, that's probably enough from me. :) > > > > Jonathan > > > > On 07/05/2019 21:04, Tim Lewis wrote: > >> Jonathan -- > >> > >> The bcfg command pre-dates the UEFI shell specification. I know of at > least two non-EDK2 implementations, including one maintained by my > company, that are implemented to the specification. S
Re: [edk2-devel] [PATCH v1 1/1] ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' option
Ah, hopefully that simplifies the decision. At any rate, thank you for checking and clarifying. For those working on the spec, in addition to clarifying -opt to avoid this confusion, one further thing to do would be to clarify the format of the "in hexadecimal" option. It would be good if it could state which of leading "0x", just leading zeros, and no prefix at all, are valid. The edk2 implementation only appeared to support leading "0x" and no prefix at all based on my previous testing, but I saw (what appeared to nowadays be incorrect) comments in the code stating that leading zeros were also valid. Jonathan On 07/05/2019 21:48, Tim Lewis wrote: > Jim -- > > Well, speaking of shooting-oneself-in-the-foot, it turns out that our > non-EDK2 implementation followed the recommendation in the patch. > > I agree that the spec is ambiguous and, it turns out that our largest use > case already uses the recommended behavior. > > Sorry to one and all. > > Tim > > -Original Message- > From: jim.dai...@dell.com > Sent: Tuesday, May 7, 2019 1:30 PM > To: devel@edk2.groups.io; tim.le...@insyde.com > Cc: dandan...@intel.com; jw...@jwatt.org; jaben.car...@intel.com; > zhichao....@intel.com; ray...@intel.com > Subject: RE: [edk2-devel] [PATCH v1 1/1] ShellPkg/UefiShellBcfgCommandLib: > Fix '-opt' option > > Tim, > > Out of curiosity, what does the specification you refer to that was used to > write the non-EDK2 implementations say about the -opt switch? > > Regards, > Jim > > -Original Message- > From: devel@edk2.groups.io On Behalf Of Tim Lewis > Sent: Tuesday, May 7, 2019 3:04 PM > To: 'Jonathan Watt'; 'Carsey, Jaben'; devel@edk2.groups.io; 'Gao, Zhichao'; > 'Ni, Ray' > Cc: 'Bi, Dandan' > Subject: Re: [edk2-devel] [PATCH v1 1/1] ShellPkg/UefiShellBcfgCommandLib: > Fix '-opt' option > > Jonathan -- > > The bcfg command pre-dates the UEFI shell specification. I know of at least > two non-EDK2 implementations, including one maintained by my company, that > are implemented to the specification. Server platforms that use the > "application" style boot options can regularly run over 10 options. > > I believe the better alternative is to add a new option in the specification > and leave the existing syntax for -opt. > > Thanks, > > Tim > > -Original Message----- > From: Jonathan Watt > Sent: Tuesday, May 7, 2019 12:06 PM > To: Carsey, Jaben ; devel@edk2.groups.io; > tim.le...@insyde.com; Gao, Zhichao ; Ni, Ray > > Cc: Bi, Dandan > Subject: Re: [edk2-devel] [PATCH v1 1/1] ShellPkg/UefiShellBcfgCommandLib: > Fix '-opt' option > > I should add, for me personally, once I noticed the inconsistency I changed > my scripts to use the "0x" prefix to avoid this real footgun. I imagine that > anyone else that may have encountered this would have done the same and so, > like me, wouldn't be affected by the change if it were to happen. > > On 07/05/2019 20:00, Jonathan Watt wrote: >> There is potential for that, but it's not certain. For it to happen >> scripts would need to be both omitting the "0x" prefix and be pass an >> option number greater than 9. The fact this very unexpected >> inconsistency (which will corrupt the wrong option when those same two >> things are true!) hasn't been reported before would seem to indicate >> this combination doesn't really happen/is rare in practice. >> >> Also, is TianoCore's bcfg the only implementation people are using? If >> there are other implementations, would this bring TianoCore's >> implementation into or out of line with them? That may impact whether the >> spec could/should change. >> >> On 07/05/2019 18:40, Carsey, Jaben wrote: >>> It will break existing scripts. Do you have such scripts in your >>> environment dependent on this parameter? >>> >>>> -Original Message- >>>> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf >>>> Of Tim Lewis >>>> Sent: Tuesday, May 07, 2019 9:20 AM >>>> To: devel@edk2.groups.io; Carsey, Jaben ; >>>> Gao, Zhichao ; Ni, Ray ; >>>> jw...@jwatt.org >>>> Cc: Bi, Dandan >>>> Subject: Re: [edk2-devel] [PATCH v1 1/1] >>>> ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' option >>>> Importance: High >>>> >>>> The question is whether this will break compatibility with existing >>>> shell scripts. In order to maintain that compatibility, it may be >>>> necessary to add a new option rather than trying
Re: [edk2-devel] [PATCH v1 1/1] ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' option
That is good news! Mostly because assuming it was decimal should make no sense to an engineer! :-) -Original Message- From: Tim Lewis Sent: Tuesday, May 7, 2019 3:49 PM To: Dailey, Jim; devel@edk2.groups.io Cc: dandan...@intel.com; jw...@jwatt.org; jaben.car...@intel.com; zhichao@intel.com; ray...@intel.com Subject: RE: [edk2-devel] [PATCH v1 1/1] ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' option Jim -- Well, speaking of shooting-oneself-in-the-foot, it turns out that our non-EDK2 implementation followed the recommendation in the patch. I agree that the spec is ambiguous and, it turns out that our largest use case already uses the recommended behavior. Sorry to one and all. Tim -Original Message- From: jim.dai...@dell.com Sent: Tuesday, May 7, 2019 1:30 PM To: devel@edk2.groups.io; tim.le...@insyde.com Cc: dandan...@intel.com; jw...@jwatt.org; jaben.car...@intel.com; zhichao@intel.com; ray...@intel.com Subject: RE: [edk2-devel] [PATCH v1 1/1] ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' option Tim, Out of curiosity, what does the specification you refer to that was used to write the non-EDK2 implementations say about the -opt switch? Regards, Jim -Original Message- From: devel@edk2.groups.io On Behalf Of Tim Lewis Sent: Tuesday, May 7, 2019 3:04 PM To: 'Jonathan Watt'; 'Carsey, Jaben'; devel@edk2.groups.io; 'Gao, Zhichao'; 'Ni, Ray' Cc: 'Bi, Dandan' Subject: Re: [edk2-devel] [PATCH v1 1/1] ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' option Jonathan -- The bcfg command pre-dates the UEFI shell specification. I know of at least two non-EDK2 implementations, including one maintained by my company, that are implemented to the specification. Server platforms that use the "application" style boot options can regularly run over 10 options. I believe the better alternative is to add a new option in the specification and leave the existing syntax for -opt. Thanks, Tim -Original Message- From: Jonathan Watt Sent: Tuesday, May 7, 2019 12:06 PM To: Carsey, Jaben ; devel@edk2.groups.io; tim.le...@insyde.com; Gao, Zhichao ; Ni, Ray Cc: Bi, Dandan Subject: Re: [edk2-devel] [PATCH v1 1/1] ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' option I should add, for me personally, once I noticed the inconsistency I changed my scripts to use the "0x" prefix to avoid this real footgun. I imagine that anyone else that may have encountered this would have done the same and so, like me, wouldn't be affected by the change if it were to happen. On 07/05/2019 20:00, Jonathan Watt wrote: > There is potential for that, but it's not certain. For it to happen > scripts would need to be both omitting the "0x" prefix and be pass an > option number greater than 9. The fact this very unexpected > inconsistency (which will corrupt the wrong option when those same two > things are true!) hasn't been reported before would seem to indicate > this combination doesn't really happen/is rare in practice. > > Also, is TianoCore's bcfg the only implementation people are using? If > there are other implementations, would this bring TianoCore's > implementation into or out of line with them? That may impact whether the > spec could/should change. > > On 07/05/2019 18:40, Carsey, Jaben wrote: >> It will break existing scripts. Do you have such scripts in your >> environment dependent on this parameter? >> >>> -Original Message- >>> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf >>> Of Tim Lewis >>> Sent: Tuesday, May 07, 2019 9:20 AM >>> To: devel@edk2.groups.io; Carsey, Jaben ; >>> Gao, Zhichao ; Ni, Ray ; >>> jw...@jwatt.org >>> Cc: Bi, Dandan >>> Subject: Re: [edk2-devel] [PATCH v1 1/1] >>> ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' option >>> Importance: High >>> >>> The question is whether this will break compatibility with existing >>> shell scripts. In order to maintain that compatibility, it may be >>> necessary to add a new option rather than trying to update an existing one. >>> >>> Tim >>> >>> -Original Message- >>> From: devel@edk2.groups.io On Behalf Of >>> Carsey, Jaben >>> Sent: Tuesday, May 7, 2019 7:36 AM >>> To: Gao, Zhichao ; devel@edk2.groups.io; Ni, >>> Ray ; jw...@jwatt.org >>> Cc: Bi, Dandan >>> Subject: Re: [edk2-devel] [PATCH v1 1/1] >>> ShellPkg/UefiShellBcfgCommandLib: >>> Fix '-opt' option >>> >>> Zhichao, >>> I can help submit errata for shell spec if needed. >>> >>> Per patch, >>> I agree. This looks good. >>> Reviewed-by: Jaben Carsey >>> >>> >>>>
Re: [edk2-devel] [PATCH v1 1/1] ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' option
Jonathan -- My apologies. I jumped because we've been bitten by shell "clarifications" in the past. As you've probably read in the other thread, it turns out that I (we) actually did agree with your interpretation of the spec in our alternate implementation and have been using it that way for 2+ years. And it didn't cause us grief with our other product which does use an EDK2-derived shell. Best regards, Tim -Original Message- From: devel@edk2.groups.io On Behalf Of Jonathan Watt Sent: Tuesday, May 7, 2019 1:51 PM To: Tim Lewis ; 'Carsey, Jaben' ; devel@edk2.groups.io; 'Gao, Zhichao' ; 'Ni, Ray' Cc: 'Bi, Dandan' Subject: Re: [edk2-devel] [PATCH v1 1/1] ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' option Hi Tim, For context, I'm just some random guy who tripped over this issue on his home workstation and thought he'd try and remove the footgun to save anyone else the same pain. I was specifically replying to the unconditional statement "It will break existing scripts." (not made by you) to provide what I hope was some qualification and balance to the face value of that statement, and to suggest some other things that should be considered. As far as deciding what the best resolution is, I'm not qualified for that. I am curious about one thing though. The sentence you wrote that ends with "that are implemented to the specification" sounds like you're saying making the proposed change would violate the specification. That does not seem to be the case from my reading, and my reading would be that it would actually make it do what most people would expect from reading the specification. Specifically, the usage block for bcfg in the specification says: Usage: bcfg driver|boot [dump [-v]] bcfg driver|boot [add # file "desc"] [addp # file “desc”] [addh # handle “desc”] bcfg driver|boot [rm #] bcfg driver|boot [mv # #] bcfg driver|boot [mod # “desc”] | [modf # file] | [modp # file] | [modh # handle] bcfg driver|boot [-opt # [[filename]|[”data”]] | [KeyData *]] It seems natural to assume from that that the "#" for all options is the "same thing" and would be handled the same way. The comment for the -opt option does not indicate otherwise: -opt Modify the optional data associated with a driver or boot option. Followed either by the filename of the file which contains the binary data to be associated with the driver or boot option optional data, or else the quote-delimited data that will be associated with the driver or boot option optional data. In fact the use of the term "driver or boot option" for -opt and the other options indicates that it is the same thing as for the other options (which explicitly say that the "#" is a hexadecimal number), even if "#" isn't described explicitly in this case. I'm glad to hear there are other implementations, because given the disagreement over what the spec intends, it would be useful to compare them and consider converging. Anyway, that's probably enough from me. :) Jonathan On 07/05/2019 21:04, Tim Lewis wrote: > Jonathan -- > > The bcfg command pre-dates the UEFI shell specification. I know of at least > two non-EDK2 implementations, including one maintained by my company, that > are implemented to the specification. Server platforms that use the > "application" style boot options can regularly run over 10 options. > > I believe the better alternative is to add a new option in the specification > and leave the existing syntax for -opt. > > Thanks, > > Tim > > -Original Message- > From: Jonathan Watt > Sent: Tuesday, May 7, 2019 12:06 PM > To: Carsey, Jaben ; devel@edk2.groups.io; > tim.le...@insyde.com; Gao, Zhichao ; Ni, Ray > > Cc: Bi, Dandan > Subject: Re: [edk2-devel] [PATCH v1 1/1] > ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' option > > I should add, for me personally, once I noticed the inconsistency I changed > my scripts to use the "0x" prefix to avoid this real footgun. I imagine that > anyone else that may have encountered this would have done the same and so, > like me, wouldn't be affected by the change if it were to happen. > > On 07/05/2019 20:00, Jonathan Watt wrote: >> There is potential for that, but it's not certain. For it to happen >> scripts would need to be both omitting the "0x" prefix and be pass an >> option number greater than 9. The fact this very unexpected >> inconsistency (which will corrupt the wrong option when those same >> two things are true!) hasn't been reported before would seem to >> indicate this combination doesn't really happen/is rare in practice. >> >> Also, is TianoCore's bcfg t
Re: [edk2-devel] [PATCH v1 1/1] ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' option
No apologies necessary! Raising compatibility concerns is very valid. As I said, I just wanted to provide some other considerations I saw to weigh in the decision. All the best, Jonathan On 07/05/2019 22:02, Tim Lewis wrote: > Jonathan -- > > My apologies. I jumped because we've been bitten by shell "clarifications" in > the past. > > As you've probably read in the other thread, it turns out that I (we) > actually did agree with your interpretation of the spec in our alternate > implementation and have been using it that way for 2+ years. And it didn't > cause us grief with our other product which does use an EDK2-derived shell. > > Best regards, > Tim > > -Original Message- > From: devel@edk2.groups.io On Behalf Of Jonathan Watt > Sent: Tuesday, May 7, 2019 1:51 PM > To: Tim Lewis ; 'Carsey, Jaben' > ; devel@edk2.groups.io; 'Gao, Zhichao' > ; 'Ni, Ray' > Cc: 'Bi, Dandan' > Subject: Re: [edk2-devel] [PATCH v1 1/1] ShellPkg/UefiShellBcfgCommandLib: > Fix '-opt' option > > Hi Tim, > > For context, I'm just some random guy who tripped over this issue on his home > workstation and thought he'd try and remove the footgun to save anyone else > the same pain. I was specifically replying to the unconditional statement "It > will break existing scripts." (not made by you) to provide what I hope was > some qualification and balance to the face value of that statement, and to > suggest some other things that should be considered. As far as deciding what > the best resolution is, I'm not qualified for that. > > I am curious about one thing though. The sentence you wrote that ends with > "that are implemented to the specification" sounds like you're saying making > the proposed change would violate the specification. That does not seem to be > the case from my reading, and my reading would be that it would actually make > it do what most people would expect from reading the specification. > > Specifically, the usage block for bcfg in the specification says: > > Usage: > bcfg driver|boot [dump [-v]] > bcfg driver|boot [add # file "desc"] [addp # file “desc”] > [addh # handle “desc”] > bcfg driver|boot [rm #] > bcfg driver|boot [mv # #] > bcfg driver|boot [mod # “desc”] | [modf # file] | [modp # file] | > [modh # handle] > bcfg driver|boot [-opt # [[filename]|[”data”]] | > [KeyData *]] > > It seems natural to assume from that that the "#" for all options is the > "same thing" and would be handled the same way. > > The comment for the -opt option does not indicate otherwise: > > -opt > Modify the optional data associated with a driver or boot option. > Followed either by the filename of the file which contains the > binary data to be associated with the driver or boot option > optional data, or else the quote-delimited data that will be > associated with the driver or boot option optional data. > > In fact the use of the term "driver or boot option" for -opt and the other > options indicates that it is the same thing as for the other options (which > explicitly say that the "#" is a hexadecimal number), even if "#" isn't > described explicitly in this case. > > I'm glad to hear there are other implementations, because given the > disagreement over what the spec intends, it would be useful to compare them > and consider converging. > > Anyway, that's probably enough from me. :) > > Jonathan > > On 07/05/2019 21:04, Tim Lewis wrote: >> Jonathan -- >> >> The bcfg command pre-dates the UEFI shell specification. I know of at least >> two non-EDK2 implementations, including one maintained by my company, that >> are implemented to the specification. Server platforms that use the >> "application" style boot options can regularly run over 10 options. >> >> I believe the better alternative is to add a new option in the >> specification and leave the existing syntax for -opt. >> >> Thanks, >> >> Tim >> >> -Original Message- >> From: Jonathan Watt >> Sent: Tuesday, May 7, 2019 12:06 PM >> To: Carsey, Jaben ; devel@edk2.groups.io; >> tim.le...@insyde.com; Gao, Zhichao ; Ni, Ray >> >> Cc: Bi, Dandan >> Subject: Re: [edk2-devel] [PATCH v1 1/1] >> ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' option >> >> I should add, for me personally, once I noticed the inconsistency I changed >> my scripts to use the "0x" prefix to avoid this real footgun. I imagine th
Re: [edk2-devel] [PATCH v1 1/1] ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' option
Tim, Out of curiosity, what does the specification you refer to that was used to write the non-EDK2 implementations say about the -opt switch? Regards, Jim -Original Message- From: devel@edk2.groups.io On Behalf Of Tim Lewis Sent: Tuesday, May 7, 2019 3:04 PM To: 'Jonathan Watt'; 'Carsey, Jaben'; devel@edk2.groups.io; 'Gao, Zhichao'; 'Ni, Ray' Cc: 'Bi, Dandan' Subject: Re: [edk2-devel] [PATCH v1 1/1] ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' option Jonathan -- The bcfg command pre-dates the UEFI shell specification. I know of at least two non-EDK2 implementations, including one maintained by my company, that are implemented to the specification. Server platforms that use the "application" style boot options can regularly run over 10 options. I believe the better alternative is to add a new option in the specification and leave the existing syntax for -opt. Thanks, Tim -Original Message- From: Jonathan Watt Sent: Tuesday, May 7, 2019 12:06 PM To: Carsey, Jaben ; devel@edk2.groups.io; tim.le...@insyde.com; Gao, Zhichao ; Ni, Ray Cc: Bi, Dandan Subject: Re: [edk2-devel] [PATCH v1 1/1] ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' option I should add, for me personally, once I noticed the inconsistency I changed my scripts to use the "0x" prefix to avoid this real footgun. I imagine that anyone else that may have encountered this would have done the same and so, like me, wouldn't be affected by the change if it were to happen. On 07/05/2019 20:00, Jonathan Watt wrote: > There is potential for that, but it's not certain. For it to happen > scripts would need to be both omitting the "0x" prefix and be pass an > option number greater than 9. The fact this very unexpected > inconsistency (which will corrupt the wrong option when those same two > things are true!) hasn't been reported before would seem to indicate > this combination doesn't really happen/is rare in practice. > > Also, is TianoCore's bcfg the only implementation people are using? If > there are other implementations, would this bring TianoCore's > implementation into or out of line with them? That may impact whether the > spec could/should change. > > On 07/05/2019 18:40, Carsey, Jaben wrote: >> It will break existing scripts. Do you have such scripts in your >> environment dependent on this parameter? >> >>> -Original Message- >>> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf >>> Of Tim Lewis >>> Sent: Tuesday, May 07, 2019 9:20 AM >>> To: devel@edk2.groups.io; Carsey, Jaben ; >>> Gao, Zhichao ; Ni, Ray ; >>> jw...@jwatt.org >>> Cc: Bi, Dandan >>> Subject: Re: [edk2-devel] [PATCH v1 1/1] >>> ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' option >>> Importance: High >>> >>> The question is whether this will break compatibility with existing >>> shell scripts. In order to maintain that compatibility, it may be >>> necessary to add a new option rather than trying to update an existing one. >>> >>> Tim >>> >>> -Original Message- >>> From: devel@edk2.groups.io On Behalf Of >>> Carsey, Jaben >>> Sent: Tuesday, May 7, 2019 7:36 AM >>> To: Gao, Zhichao ; devel@edk2.groups.io; Ni, >>> Ray ; jw...@jwatt.org >>> Cc: Bi, Dandan >>> Subject: Re: [edk2-devel] [PATCH v1 1/1] >>> ShellPkg/UefiShellBcfgCommandLib: >>> Fix '-opt' option >>> >>> Zhichao, >>> I can help submit errata for shell spec if needed. >>> >>> Per patch, >>> I agree. This looks good. >>> Reviewed-by: Jaben Carsey >>> >>> >>>> -Original Message- >>>> From: Gao, Zhichao >>>> Sent: Tuesday, May 07, 2019 2:52 AM >>>> To: devel@edk2.groups.io; Ni, Ray ; >>>> jw...@jwatt.org >>>> Cc: Carsey, Jaben ; Bi, Dandan >>>> >>>> Subject: RE: [edk2-devel] [PATCH v1 1/1] >>>> ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' option >>>> Importance: High >>>> >>>> This patch looks good for me. >>>> Reviewed-by: Zhichao Gao >>>> >>>> But when I view the command in UEFI SHELL 2.2 spec: >>>> ... >>>> bcfg driver|boot [-opt # [[filename]|["data"]] | [KeyData >>> UnicodeChar>*]] >>>> ... >>>> -opt >>>> Modify the optional data associated with a driver or boot option. >>>> Followed either by the filename of the file which contains the >>>> binary data to be associated with the driver or boot option >>
Re: [edk2-devel] [PATCH v1 1/1] ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' option
Hi Tim, For context, I'm just some random guy who tripped over this issue on his home workstation and thought he'd try and remove the footgun to save anyone else the same pain. I was specifically replying to the unconditional statement "It will break existing scripts." (not made by you) to provide what I hope was some qualification and balance to the face value of that statement, and to suggest some other things that should be considered. As far as deciding what the best resolution is, I'm not qualified for that. I am curious about one thing though. The sentence you wrote that ends with "that are implemented to the specification" sounds like you're saying making the proposed change would violate the specification. That does not seem to be the case from my reading, and my reading would be that it would actually make it do what most people would expect from reading the specification. Specifically, the usage block for bcfg in the specification says: Usage: bcfg driver|boot [dump [-v]] bcfg driver|boot [add # file "desc"] [addp # file “desc”] [addh # handle “desc”] bcfg driver|boot [rm #] bcfg driver|boot [mv # #] bcfg driver|boot [mod # “desc”] | [modf # file] | [modp # file] | [modh # handle] bcfg driver|boot [-opt # [[filename]|[”data”]] | [KeyData *]] It seems natural to assume from that that the "#" for all options is the "same thing" and would be handled the same way. The comment for the -opt option does not indicate otherwise: -opt Modify the optional data associated with a driver or boot option. Followed either by the filename of the file which contains the binary data to be associated with the driver or boot option optional data, or else the quote-delimited data that will be associated with the driver or boot option optional data. In fact the use of the term "driver or boot option" for -opt and the other options indicates that it is the same thing as for the other options (which explicitly say that the "#" is a hexadecimal number), even if "#" isn't described explicitly in this case. I'm glad to hear there are other implementations, because given the disagreement over what the spec intends, it would be useful to compare them and consider converging. Anyway, that's probably enough from me. :) Jonathan On 07/05/2019 21:04, Tim Lewis wrote: > Jonathan -- > > The bcfg command pre-dates the UEFI shell specification. I know of at least > two non-EDK2 implementations, including one maintained by my company, that > are implemented to the specification. Server platforms that use the > "application" style boot options can regularly run over 10 options. > > I believe the better alternative is to add a new option in the specification > and leave the existing syntax for -opt. > > Thanks, > > Tim > > -Original Message- > From: Jonathan Watt > Sent: Tuesday, May 7, 2019 12:06 PM > To: Carsey, Jaben ; devel@edk2.groups.io; > tim.le...@insyde.com; Gao, Zhichao ; Ni, Ray > > Cc: Bi, Dandan > Subject: Re: [edk2-devel] [PATCH v1 1/1] ShellPkg/UefiShellBcfgCommandLib: > Fix '-opt' option > > I should add, for me personally, once I noticed the inconsistency I changed > my scripts to use the "0x" prefix to avoid this real footgun. I imagine that > anyone else that may have encountered this would have done the same and so, > like me, wouldn't be affected by the change if it were to happen. > > On 07/05/2019 20:00, Jonathan Watt wrote: >> There is potential for that, but it's not certain. For it to happen >> scripts would need to be both omitting the "0x" prefix and be pass an >> option number greater than 9. The fact this very unexpected >> inconsistency (which will corrupt the wrong option when those same two >> things are true!) hasn't been reported before would seem to indicate >> this combination doesn't really happen/is rare in practice. >> >> Also, is TianoCore's bcfg the only implementation people are using? If >> there are other implementations, would this bring TianoCore's >> implementation into or out of line with them? That may impact whether the >> spec could/should change. >> >> On 07/05/2019 18:40, Carsey, Jaben wrote: >>> It will break existing scripts. Do you have such scripts in your >>> environment dependent on this parameter? >>> >>>> -----Original Message----- >>>> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf >>>> Of Tim Lewis >>>> Sent: Tuesday, May 07, 2019 9:20 AM >>>> To: devel@edk2.groups.io; Carsey, Jaben ; >>>> Gao, Zhichao ; Ni, Ray ; >
Re: [edk2-devel] [PATCH v1 1/1] ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' option
Jim -- Well, speaking of shooting-oneself-in-the-foot, it turns out that our non-EDK2 implementation followed the recommendation in the patch. I agree that the spec is ambiguous and, it turns out that our largest use case already uses the recommended behavior. Sorry to one and all. Tim -Original Message- From: jim.dai...@dell.com Sent: Tuesday, May 7, 2019 1:30 PM To: devel@edk2.groups.io; tim.le...@insyde.com Cc: dandan...@intel.com; jw...@jwatt.org; jaben.car...@intel.com; zhichao@intel.com; ray...@intel.com Subject: RE: [edk2-devel] [PATCH v1 1/1] ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' option Tim, Out of curiosity, what does the specification you refer to that was used to write the non-EDK2 implementations say about the -opt switch? Regards, Jim -Original Message- From: devel@edk2.groups.io On Behalf Of Tim Lewis Sent: Tuesday, May 7, 2019 3:04 PM To: 'Jonathan Watt'; 'Carsey, Jaben'; devel@edk2.groups.io; 'Gao, Zhichao'; 'Ni, Ray' Cc: 'Bi, Dandan' Subject: Re: [edk2-devel] [PATCH v1 1/1] ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' option Jonathan -- The bcfg command pre-dates the UEFI shell specification. I know of at least two non-EDK2 implementations, including one maintained by my company, that are implemented to the specification. Server platforms that use the "application" style boot options can regularly run over 10 options. I believe the better alternative is to add a new option in the specification and leave the existing syntax for -opt. Thanks, Tim -Original Message- From: Jonathan Watt Sent: Tuesday, May 7, 2019 12:06 PM To: Carsey, Jaben ; devel@edk2.groups.io; tim.le...@insyde.com; Gao, Zhichao ; Ni, Ray Cc: Bi, Dandan Subject: Re: [edk2-devel] [PATCH v1 1/1] ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' option I should add, for me personally, once I noticed the inconsistency I changed my scripts to use the "0x" prefix to avoid this real footgun. I imagine that anyone else that may have encountered this would have done the same and so, like me, wouldn't be affected by the change if it were to happen. On 07/05/2019 20:00, Jonathan Watt wrote: > There is potential for that, but it's not certain. For it to happen > scripts would need to be both omitting the "0x" prefix and be pass an > option number greater than 9. The fact this very unexpected > inconsistency (which will corrupt the wrong option when those same two > things are true!) hasn't been reported before would seem to indicate > this combination doesn't really happen/is rare in practice. > > Also, is TianoCore's bcfg the only implementation people are using? If > there are other implementations, would this bring TianoCore's > implementation into or out of line with them? That may impact whether the > spec could/should change. > > On 07/05/2019 18:40, Carsey, Jaben wrote: >> It will break existing scripts. Do you have such scripts in your >> environment dependent on this parameter? >> >>> -Original Message- >>> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf >>> Of Tim Lewis >>> Sent: Tuesday, May 07, 2019 9:20 AM >>> To: devel@edk2.groups.io; Carsey, Jaben ; >>> Gao, Zhichao ; Ni, Ray ; >>> jw...@jwatt.org >>> Cc: Bi, Dandan >>> Subject: Re: [edk2-devel] [PATCH v1 1/1] >>> ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' option >>> Importance: High >>> >>> The question is whether this will break compatibility with existing >>> shell scripts. In order to maintain that compatibility, it may be >>> necessary to add a new option rather than trying to update an existing one. >>> >>> Tim >>> >>> -Original Message- >>> From: devel@edk2.groups.io On Behalf Of >>> Carsey, Jaben >>> Sent: Tuesday, May 7, 2019 7:36 AM >>> To: Gao, Zhichao ; devel@edk2.groups.io; Ni, >>> Ray ; jw...@jwatt.org >>> Cc: Bi, Dandan >>> Subject: Re: [edk2-devel] [PATCH v1 1/1] >>> ShellPkg/UefiShellBcfgCommandLib: >>> Fix '-opt' option >>> >>> Zhichao, >>> I can help submit errata for shell spec if needed. >>> >>> Per patch, >>> I agree. This looks good. >>> Reviewed-by: Jaben Carsey >>> >>> >>>> -Original Message- >>>> From: Gao, Zhichao >>>> Sent: Tuesday, May 07, 2019 2:52 AM >>>> To: devel@edk2.groups.io; Ni, Ray ; >>>> jw...@jwatt.org >>>> Cc: Carsey, Jaben ; Bi, Dandan >>>> >>>> Subject: RE: [edk2-devel] [PATCH v1 1/1] >>>> ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' option >>>> Importance
Re: [edk2-devel] [PATCH v1 1/1] ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' option
Jonathan -- The bcfg command pre-dates the UEFI shell specification. I know of at least two non-EDK2 implementations, including one maintained by my company, that are implemented to the specification. Server platforms that use the "application" style boot options can regularly run over 10 options. I believe the better alternative is to add a new option in the specification and leave the existing syntax for -opt. Thanks, Tim -Original Message- From: Jonathan Watt Sent: Tuesday, May 7, 2019 12:06 PM To: Carsey, Jaben ; devel@edk2.groups.io; tim.le...@insyde.com; Gao, Zhichao ; Ni, Ray Cc: Bi, Dandan Subject: Re: [edk2-devel] [PATCH v1 1/1] ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' option I should add, for me personally, once I noticed the inconsistency I changed my scripts to use the "0x" prefix to avoid this real footgun. I imagine that anyone else that may have encountered this would have done the same and so, like me, wouldn't be affected by the change if it were to happen. On 07/05/2019 20:00, Jonathan Watt wrote: > There is potential for that, but it's not certain. For it to happen > scripts would need to be both omitting the "0x" prefix and be pass an > option number greater than 9. The fact this very unexpected > inconsistency (which will corrupt the wrong option when those same two > things are true!) hasn't been reported before would seem to indicate > this combination doesn't really happen/is rare in practice. > > Also, is TianoCore's bcfg the only implementation people are using? If > there are other implementations, would this bring TianoCore's > implementation into or out of line with them? That may impact whether the > spec could/should change. > > On 07/05/2019 18:40, Carsey, Jaben wrote: >> It will break existing scripts. Do you have such scripts in your >> environment dependent on this parameter? >> >>> -Original Message- >>> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf >>> Of Tim Lewis >>> Sent: Tuesday, May 07, 2019 9:20 AM >>> To: devel@edk2.groups.io; Carsey, Jaben ; >>> Gao, Zhichao ; Ni, Ray ; >>> jw...@jwatt.org >>> Cc: Bi, Dandan >>> Subject: Re: [edk2-devel] [PATCH v1 1/1] >>> ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' option >>> Importance: High >>> >>> The question is whether this will break compatibility with existing >>> shell scripts. In order to maintain that compatibility, it may be >>> necessary to add a new option rather than trying to update an existing one. >>> >>> Tim >>> >>> -Original Message- >>> From: devel@edk2.groups.io On Behalf Of >>> Carsey, Jaben >>> Sent: Tuesday, May 7, 2019 7:36 AM >>> To: Gao, Zhichao ; devel@edk2.groups.io; Ni, >>> Ray ; jw...@jwatt.org >>> Cc: Bi, Dandan >>> Subject: Re: [edk2-devel] [PATCH v1 1/1] >>> ShellPkg/UefiShellBcfgCommandLib: >>> Fix '-opt' option >>> >>> Zhichao, >>> I can help submit errata for shell spec if needed. >>> >>> Per patch, >>> I agree. This looks good. >>> Reviewed-by: Jaben Carsey >>> >>> >>>> -Original Message- >>>> From: Gao, Zhichao >>>> Sent: Tuesday, May 07, 2019 2:52 AM >>>> To: devel@edk2.groups.io; Ni, Ray ; >>>> jw...@jwatt.org >>>> Cc: Carsey, Jaben ; Bi, Dandan >>>> >>>> Subject: RE: [edk2-devel] [PATCH v1 1/1] >>>> ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' option >>>> Importance: High >>>> >>>> This patch looks good for me. >>>> Reviewed-by: Zhichao Gao >>>> >>>> But when I view the command in UEFI SHELL 2.2 spec: >>>> ... >>>> bcfg driver|boot [-opt # [[filename]|["data"]] | [KeyData >>> UnicodeChar>*]] >>>> ... >>>> -opt >>>> Modify the optional data associated with a driver or boot option. >>>> Followed either by the filename of the file which contains the >>>> binary data to be associated with the driver or boot option >>>> optional data, or else the quote- delimited data that will be >>>> associated with the driver or boot option optional data. >>>> ... >>>> >>>> This description lack the comment of '#' parameter and that may >>>> make the consumer confused. Usually consumers would regard it as >>>> the same in other option, such as ' bcfg driver|boot [rm #]'. The >>>> '#' is clearly
Re: [edk2-devel] [PATCH v1 1/1] ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' option
There is potential for that, but it's not certain. For it to happen scripts would need to be both omitting the "0x" prefix and be pass an option number greater than 9. The fact this very unexpected inconsistency (which will corrupt the wrong option when those same two things are true!) hasn't been reported before would seem to indicate this combination doesn't really happen/is rare in practice. Also, is TianoCore's bcfg the only implementation people are using? If there are other implementations, would this bring TianoCore's implementation into or out of line with them? That may impact whether the spec could/should change. On 07/05/2019 18:40, Carsey, Jaben wrote: > It will break existing scripts. Do you have such scripts in your environment > dependent on this parameter? > >> -Original Message- >> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of >> Tim Lewis >> Sent: Tuesday, May 07, 2019 9:20 AM >> To: devel@edk2.groups.io; Carsey, Jaben ; Gao, >> Zhichao ; Ni, Ray ; >> jw...@jwatt.org >> Cc: Bi, Dandan >> Subject: Re: [edk2-devel] [PATCH v1 1/1] >> ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' option >> Importance: High >> >> The question is whether this will break compatibility with existing shell >> scripts. In order to maintain that compatibility, it may be necessary to add >> a new option rather than trying to update an existing one. >> >> Tim >> >> -Original Message- >> From: devel@edk2.groups.io On Behalf Of Carsey, >> Jaben >> Sent: Tuesday, May 7, 2019 7:36 AM >> To: Gao, Zhichao ; devel@edk2.groups.io; Ni, Ray >> ; jw...@jwatt.org >> Cc: Bi, Dandan >> Subject: Re: [edk2-devel] [PATCH v1 1/1] >> ShellPkg/UefiShellBcfgCommandLib: >> Fix '-opt' option >> >> Zhichao, >> I can help submit errata for shell spec if needed. >> >> Per patch, >> I agree. This looks good. >> Reviewed-by: Jaben Carsey >> >> >>> -Original Message----- >>> From: Gao, Zhichao >>> Sent: Tuesday, May 07, 2019 2:52 AM >>> To: devel@edk2.groups.io; Ni, Ray ; jw...@jwatt.org >>> Cc: Carsey, Jaben ; Bi, Dandan >>> >>> Subject: RE: [edk2-devel] [PATCH v1 1/1] >>> ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' option >>> Importance: High >>> >>> This patch looks good for me. >>> Reviewed-by: Zhichao Gao >>> >>> But when I view the command in UEFI SHELL 2.2 spec: >>> ... >>> bcfg driver|boot [-opt # [[filename]|["data"]] | [KeyData >> UnicodeChar>*]] >>> ... >>> -opt >>> Modify the optional data associated with a driver or boot option. >>> Followed either by the filename of the file which contains the binary >>> data to be associated with the driver or boot option optional data, or >>> else the quote- delimited data that will be associated with the driver >>> or boot option optional data. >>> ... >>> >>> This description lack the comment of '#' parameter and that may make >>> the consumer confused. Usually consumers would regard it as the same >>> in other option, such as ' bcfg driver|boot [rm #]'. The '#' is >>> clearly descripted as a hexadecimal parameter: >>> rm >>> Remove an option. The # parameter lists the option number to remove in >>> hexadecimal. >>> >>> So I think we should update the shell spec by the way. >>> >>> Thanks, >>> Zhichao >>> >>>> -Original Message- >>>> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf >>>> Of >>> Ni, >>>> Ray >>>> Sent: Monday, May 6, 2019 10:02 PM >>>> To: jw...@jwatt.org; devel@edk2.groups.io >>>> Cc: Carsey, Jaben ; Bi, Dandan >>>> >>>> Subject: Re: [edk2-devel] [PATCH v1 1/1] >>> ShellPkg/UefiShellBcfgCommandLib: >>>> Fix '-opt' option >>>> >>>> Dandan, >>>> Can you please help to review? >>>> >>>> Thanks, >>>> Ray >>>> >>>>> -Original Message- >>>>> From: jw...@jwatt.org [mailto:jw...@jwatt.org] >>>>> Sent: Monday, May 6, 2019 9:03 PM >>>>> To: devel@edk2.groups.io >>>>> Cc: Carsey, Jaben ; Ni, Ray >>>>> >>>>> Subject: [PATCH v1 1/1] ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' >>>>> option >>>>> >>>>> From: Jonathan Watt >>>
Re: [edk2-devel] [PATCH v1 1/1] ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' option
It will break existing scripts. Do you have such scripts in your environment dependent on this parameter? > -Original Message- > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of > Tim Lewis > Sent: Tuesday, May 07, 2019 9:20 AM > To: devel@edk2.groups.io; Carsey, Jaben ; Gao, > Zhichao ; Ni, Ray ; > jw...@jwatt.org > Cc: Bi, Dandan > Subject: Re: [edk2-devel] [PATCH v1 1/1] > ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' option > Importance: High > > The question is whether this will break compatibility with existing shell > scripts. In order to maintain that compatibility, it may be necessary to add > a new option rather than trying to update an existing one. > > Tim > > -Original Message- > From: devel@edk2.groups.io On Behalf Of Carsey, > Jaben > Sent: Tuesday, May 7, 2019 7:36 AM > To: Gao, Zhichao ; devel@edk2.groups.io; Ni, Ray > ; jw...@jwatt.org > Cc: Bi, Dandan > Subject: Re: [edk2-devel] [PATCH v1 1/1] > ShellPkg/UefiShellBcfgCommandLib: > Fix '-opt' option > > Zhichao, > I can help submit errata for shell spec if needed. > > Per patch, > I agree. This looks good. > Reviewed-by: Jaben Carsey > > > > -Original Message- > > From: Gao, Zhichao > > Sent: Tuesday, May 07, 2019 2:52 AM > > To: devel@edk2.groups.io; Ni, Ray ; jw...@jwatt.org > > Cc: Carsey, Jaben ; Bi, Dandan > > > > Subject: RE: [edk2-devel] [PATCH v1 1/1] > > ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' option > > Importance: High > > > > This patch looks good for me. > > Reviewed-by: Zhichao Gao > > > > But when I view the command in UEFI SHELL 2.2 spec: > > ... > > bcfg driver|boot [-opt # [[filename]|["data"]] | [KeyData > UnicodeChar>*]] > > ... > > -opt > > Modify the optional data associated with a driver or boot option. > > Followed either by the filename of the file which contains the binary > > data to be associated with the driver or boot option optional data, or > > else the quote- delimited data that will be associated with the driver > > or boot option optional data. > > ... > > > > This description lack the comment of '#' parameter and that may make > > the consumer confused. Usually consumers would regard it as the same > > in other option, such as ' bcfg driver|boot [rm #]'. The '#' is > > clearly descripted as a hexadecimal parameter: > > rm > > Remove an option. The # parameter lists the option number to remove in > > hexadecimal. > > > > So I think we should update the shell spec by the way. > > > > Thanks, > > Zhichao > > > > > -Original Message- > > > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf > > > Of > > Ni, > > > Ray > > > Sent: Monday, May 6, 2019 10:02 PM > > > To: jw...@jwatt.org; devel@edk2.groups.io > > > Cc: Carsey, Jaben ; Bi, Dandan > > > > > > Subject: Re: [edk2-devel] [PATCH v1 1/1] > > ShellPkg/UefiShellBcfgCommandLib: > > > Fix '-opt' option > > > > > > Dandan, > > > Can you please help to review? > > > > > > Thanks, > > > Ray > > > > > > > -Original Message- > > > > From: jw...@jwatt.org [mailto:jw...@jwatt.org] > > > > Sent: Monday, May 6, 2019 9:03 PM > > > > To: devel@edk2.groups.io > > > > Cc: Carsey, Jaben ; Ni, Ray > > > > > > > > Subject: [PATCH v1 1/1] ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' > > > > option > > > > > > > > From: Jonathan Watt > > > > > > > > For all other bcfg commands the "#" (option number) argument(s) > > > > are treated as hexedecimal values regardless of whether or not > > > > they are prefixed by "0x". This change fixes '-opt' to handle its "#" > > > > (option number) argument consistently with the other commands. > > > > > > > > Making this change removes a potential footgun whereby a user that > > > > has been using a number without a "0x" prefix with other bcfg > > > > commands finds that, on using that exact same number with '-opt', > > > > it has this time unexpectedly been interpreted as a decimal number > > > > and they have modified > > > > (corrupted) an unrelated load option. For example, a user may > > > > have been specifying "10" to other commands to have them act on > > > > the 16th option (beca
Re: [edk2-devel] [PATCH v1 1/1] ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' option
Yes. And we have been recommending the usage of bcfg to our customers for years. Tim -Original Message- From: Carsey, Jaben Sent: Tuesday, May 7, 2019 10:41 AM To: devel@edk2.groups.io; tim.le...@insyde.com; Gao, Zhichao ; Ni, Ray ; jw...@jwatt.org Cc: Bi, Dandan Subject: RE: [edk2-devel] [PATCH v1 1/1] ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' option It will break existing scripts. Do you have such scripts in your environment dependent on this parameter? > -Original Message- > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of > Tim Lewis > Sent: Tuesday, May 07, 2019 9:20 AM > To: devel@edk2.groups.io; Carsey, Jaben ; Gao, > Zhichao ; Ni, Ray ; > jw...@jwatt.org > Cc: Bi, Dandan > Subject: Re: [edk2-devel] [PATCH v1 1/1] > ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' option > Importance: High > > The question is whether this will break compatibility with existing > shell scripts. In order to maintain that compatibility, it may be > necessary to add a new option rather than trying to update an existing one. > > Tim > > -Original Message- > From: devel@edk2.groups.io On Behalf Of Carsey, > Jaben > Sent: Tuesday, May 7, 2019 7:36 AM > To: Gao, Zhichao ; devel@edk2.groups.io; Ni, > Ray ; jw...@jwatt.org > Cc: Bi, Dandan > Subject: Re: [edk2-devel] [PATCH v1 1/1] > ShellPkg/UefiShellBcfgCommandLib: > Fix '-opt' option > > Zhichao, > I can help submit errata for shell spec if needed. > > Per patch, > I agree. This looks good. > Reviewed-by: Jaben Carsey > > > > -Original Message- > > From: Gao, Zhichao > > Sent: Tuesday, May 07, 2019 2:52 AM > > To: devel@edk2.groups.io; Ni, Ray ; > > jw...@jwatt.org > > Cc: Carsey, Jaben ; Bi, Dandan > > > > Subject: RE: [edk2-devel] [PATCH v1 1/1] > > ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' option > > Importance: High > > > > This patch looks good for me. > > Reviewed-by: Zhichao Gao > > > > But when I view the command in UEFI SHELL 2.2 spec: > > ... > > bcfg driver|boot [-opt # [[filename]|["data"]] | [KeyData > UnicodeChar>*]] > > ... > > -opt > > Modify the optional data associated with a driver or boot option. > > Followed either by the filename of the file which contains the > > binary data to be associated with the driver or boot option optional > > data, or else the quote- delimited data that will be associated with > > the driver or boot option optional data. > > ... > > > > This description lack the comment of '#' parameter and that may make > > the consumer confused. Usually consumers would regard it as the same > > in other option, such as ' bcfg driver|boot [rm #]'. The '#' is > > clearly descripted as a hexadecimal parameter: > > rm > > Remove an option. The # parameter lists the option number to remove > > in hexadecimal. > > > > So I think we should update the shell spec by the way. > > > > Thanks, > > Zhichao > > > > > -Original Message- > > > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf > > > Of > > Ni, > > > Ray > > > Sent: Monday, May 6, 2019 10:02 PM > > > To: jw...@jwatt.org; devel@edk2.groups.io > > > Cc: Carsey, Jaben ; Bi, Dandan > > > > > > Subject: Re: [edk2-devel] [PATCH v1 1/1] > > ShellPkg/UefiShellBcfgCommandLib: > > > Fix '-opt' option > > > > > > Dandan, > > > Can you please help to review? > > > > > > Thanks, > > > Ray > > > > > > > -Original Message- > > > > From: jw...@jwatt.org [mailto:jw...@jwatt.org] > > > > Sent: Monday, May 6, 2019 9:03 PM > > > > To: devel@edk2.groups.io > > > > Cc: Carsey, Jaben ; Ni, Ray > > > > > > > > Subject: [PATCH v1 1/1] ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' > > > > option > > > > > > > > From: Jonathan Watt > > > > > > > > For all other bcfg commands the "#" (option number) argument(s) > > > > are treated as hexedecimal values regardless of whether or not > > > > they are prefixed by "0x". This change fixes '-opt' to handle its "#" > > > > (option number) argument consistently with the other commands. > > > > > > > > Making this change removes a potential footgun whereby a user > > > > that has been using a number without a "0x" prefix with other > > > > bcfg commands finds tha
Re: [edk2-devel] [PATCH v1 1/1] ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' option
The question is whether this will break compatibility with existing shell scripts. In order to maintain that compatibility, it may be necessary to add a new option rather than trying to update an existing one. Tim -Original Message- From: devel@edk2.groups.io On Behalf Of Carsey, Jaben Sent: Tuesday, May 7, 2019 7:36 AM To: Gao, Zhichao ; devel@edk2.groups.io; Ni, Ray ; jw...@jwatt.org Cc: Bi, Dandan Subject: Re: [edk2-devel] [PATCH v1 1/1] ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' option Zhichao, I can help submit errata for shell spec if needed. Per patch, I agree. This looks good. Reviewed-by: Jaben Carsey > -Original Message- > From: Gao, Zhichao > Sent: Tuesday, May 07, 2019 2:52 AM > To: devel@edk2.groups.io; Ni, Ray ; jw...@jwatt.org > Cc: Carsey, Jaben ; Bi, Dandan > > Subject: RE: [edk2-devel] [PATCH v1 1/1] > ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' option > Importance: High > > This patch looks good for me. > Reviewed-by: Zhichao Gao > > But when I view the command in UEFI SHELL 2.2 spec: > ... > bcfg driver|boot [-opt # [[filename]|["data"]] | [KeyData UnicodeChar>*]] > ... > -opt > Modify the optional data associated with a driver or boot option. > Followed either by the filename of the file which contains the binary > data to be associated with the driver or boot option optional data, or > else the quote- delimited data that will be associated with the driver > or boot option optional data. > ... > > This description lack the comment of '#' parameter and that may make > the consumer confused. Usually consumers would regard it as the same > in other option, such as ' bcfg driver|boot [rm #]'. The '#' is > clearly descripted as a hexadecimal parameter: > rm > Remove an option. The # parameter lists the option number to remove in > hexadecimal. > > So I think we should update the shell spec by the way. > > Thanks, > Zhichao > > > -Original Message- > > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf > > Of > Ni, > > Ray > > Sent: Monday, May 6, 2019 10:02 PM > > To: jw...@jwatt.org; devel@edk2.groups.io > > Cc: Carsey, Jaben ; Bi, Dandan > > > > Subject: Re: [edk2-devel] [PATCH v1 1/1] > ShellPkg/UefiShellBcfgCommandLib: > > Fix '-opt' option > > > > Dandan, > > Can you please help to review? > > > > Thanks, > > Ray > > > > > -Original Message- > > > From: jw...@jwatt.org [mailto:jw...@jwatt.org] > > > Sent: Monday, May 6, 2019 9:03 PM > > > To: devel@edk2.groups.io > > > Cc: Carsey, Jaben ; Ni, Ray > > > > > > Subject: [PATCH v1 1/1] ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' > > > option > > > > > > From: Jonathan Watt > > > > > > For all other bcfg commands the "#" (option number) argument(s) > > > are treated as hexedecimal values regardless of whether or not > > > they are prefixed by "0x". This change fixes '-opt' to handle its "#" > > > (option number) argument consistently with the other commands. > > > > > > Making this change removes a potential footgun whereby a user that > > > has been using a number without a "0x" prefix with other bcfg > > > commands finds that, on using that exact same number with '-opt', > > > it has this time unexpectedly been interpreted as a decimal number > > > and they have modified > > > (corrupted) an unrelated load option. For example, a user may > > > have been specifying "10" to other commands to have them act on > > > the 16th option (because simply "10", without any prefix, is how > > > 'bcfg boot dump' displayed the option number for the 16th option). > > > Unfortunately for them, if they also use '-opt' with "10" it would > > > unexpectedly and inconsistently act on the 10th option. > > > > > > CC: Jaben Carsey > > > CC: Ray Ni > > > Signed-off-by: Jonathan Watt > > > --- > > > > > > ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.c > | > > > 2 > > > +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git > > > > a/ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.c > > > > b/ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.c > > > index d033c7c1dc59..e8b48b4990dd 100644 > > > --- > > > > a/ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.c > &
Re: [edk2-devel] [PATCH v1 1/1] ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' option
Also agree to update Shell Spec to add description for "#" in "-opt" part to make it consistent with other options since this patch has updated the code behavior to be consistent. For the patch Reviewed-by: Bi Dandan Thanks, Dandan > -Original Message- > From: Carsey, Jaben > Sent: Tuesday, May 07, 2019 10:36 PM > To: Gao, Zhichao ; devel@edk2.groups.io; Ni, Ray > ; jw...@jwatt.org > Cc: Bi, Dandan > Subject: RE: [edk2-devel] [PATCH v1 1/1] > ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' option > > Zhichao, > I can help submit errata for shell spec if needed. > > Per patch, > I agree. This looks good. > Reviewed-by: Jaben Carsey > > > > -Original Message- > > From: Gao, Zhichao > > Sent: Tuesday, May 07, 2019 2:52 AM > > To: devel@edk2.groups.io; Ni, Ray ; jw...@jwatt.org > > Cc: Carsey, Jaben ; Bi, Dandan > > > > Subject: RE: [edk2-devel] [PATCH v1 1/1] > > ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' option > > Importance: High > > > > This patch looks good for me. > > Reviewed-by: Zhichao Gao > > > > But when I view the command in UEFI SHELL 2.2 spec: > > ... > > bcfg driver|boot [-opt # [[filename]|["data"]] | [KeyData > UnicodeChar>*]] > > ... > > -opt > > Modify the optional data associated with a driver or boot option. > > Followed either by the filename of the file which contains the binary > > data to be associated with the driver or boot option optional data, or > > else the quote- delimited data that will be associated with the driver > > or boot option optional data. > > ... > > > > This description lack the comment of '#' parameter and that may make > > the consumer confused. Usually consumers would regard it as the same > > in other option, such as ' bcfg driver|boot [rm #]'. The '#' is > > clearly descripted as a hexadecimal parameter: > > rm > > Remove an option. The # parameter lists the option number to remove in > > hexadecimal. > > > > So I think we should update the shell spec by the way. > > > > Thanks, > > Zhichao > > > > > -Original Message- > > > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf > > > Of > > Ni, > > > Ray > > > Sent: Monday, May 6, 2019 10:02 PM > > > To: jw...@jwatt.org; devel@edk2.groups.io > > > Cc: Carsey, Jaben ; Bi, Dandan > > > > > > Subject: Re: [edk2-devel] [PATCH v1 1/1] > > ShellPkg/UefiShellBcfgCommandLib: > > > Fix '-opt' option > > > > > > Dandan, > > > Can you please help to review? > > > > > > Thanks, > > > Ray > > > > > > > -Original Message- > > > > From: jw...@jwatt.org [mailto:jw...@jwatt.org] > > > > Sent: Monday, May 6, 2019 9:03 PM > > > > To: devel@edk2.groups.io > > > > Cc: Carsey, Jaben ; Ni, Ray > > > > > > > > Subject: [PATCH v1 1/1] ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' > > > > option > > > > > > > > From: Jonathan Watt > > > > > > > > For all other bcfg commands the "#" (option number) argument(s) > > > > are treated as hexedecimal values regardless of whether or not > > > > they are prefixed by "0x". This change fixes '-opt' to handle its "#" > > > > (option number) argument consistently with the other commands. > > > > > > > > Making this change removes a potential footgun whereby a user that > > > > has been using a number without a "0x" prefix with other bcfg > > > > commands finds that, on using that exact same number with '-opt', > > > > it has this time unexpectedly been interpreted as a decimal number > > > > and they have modified > > > > (corrupted) an unrelated load option. For example, a user may > > > > have been specifying "10" to other commands to have them act on > > > > the 16th option (because simply "10", without any prefix, is how > > > > 'bcfg boot dump' displayed the option number for the 16th option). > > > > Unfortunately for them, if they also use '-opt' with "10" it would > > > > unexpectedly and inconsistently act on the 10th option. > > > > > > > > CC: Jaben Carsey > > > > CC: Ray Ni > > > > Signed-off-by: Jonathan Watt > > > > --- > > > > > > > > > ShellPkg/Library/UefiShellBcfgCom
Re: [edk2-devel] [PATCH v1 1/1] ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' option
Zhichao,
I can help submit errata for shell spec if needed.
Per patch,
I agree. This looks good.
Reviewed-by: Jaben Carsey
> -Original Message-
> From: Gao, Zhichao
> Sent: Tuesday, May 07, 2019 2:52 AM
> To: devel@edk2.groups.io; Ni, Ray ; jw...@jwatt.org
> Cc: Carsey, Jaben ; Bi, Dandan
>
> Subject: RE: [edk2-devel] [PATCH v1 1/1]
> ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' option
> Importance: High
>
> This patch looks good for me.
> Reviewed-by: Zhichao Gao
>
> But when I view the command in UEFI SHELL 2.2 spec:
> ...
> bcfg driver|boot [-opt # [[filename]|["data"]] | [KeyData UnicodeChar>*]]
> ...
> -opt
> Modify the optional data associated with a driver or boot option. Followed
> either by the filename of the file which contains the binary data to be
> associated with the driver or boot option optional data, or else the quote-
> delimited data that will be associated with the driver or boot option optional
> data.
> ...
>
> This description lack the comment of '#' parameter and that may make the
> consumer confused. Usually consumers would regard it as the same in other
> option, such as ' bcfg driver|boot [rm #]'. The '#' is clearly descripted as a
> hexadecimal parameter:
> rm
> Remove an option. The # parameter lists the option number to remove in
> hexadecimal.
>
> So I think we should update the shell spec by the way.
>
> Thanks,
> Zhichao
>
> > -Original Message-
> > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Ni,
> > Ray
> > Sent: Monday, May 6, 2019 10:02 PM
> > To: jw...@jwatt.org; devel@edk2.groups.io
> > Cc: Carsey, Jaben ; Bi, Dandan
> >
> > Subject: Re: [edk2-devel] [PATCH v1 1/1]
> ShellPkg/UefiShellBcfgCommandLib:
> > Fix '-opt' option
> >
> > Dandan,
> > Can you please help to review?
> >
> > Thanks,
> > Ray
> >
> > > -Original Message-
> > > From: jw...@jwatt.org [mailto:jw...@jwatt.org]
> > > Sent: Monday, May 6, 2019 9:03 PM
> > > To: devel@edk2.groups.io
> > > Cc: Carsey, Jaben ; Ni, Ray
> > > Subject: [PATCH v1 1/1] ShellPkg/UefiShellBcfgCommandLib: Fix '-opt'
> > > option
> > >
> > > From: Jonathan Watt
> > >
> > > For all other bcfg commands the "#" (option number) argument(s) are
> > > treated as hexedecimal values regardless of whether or not they are
> > > prefixed by "0x". This change fixes '-opt' to handle its "#"
> > > (option number) argument consistently with the other commands.
> > >
> > > Making this change removes a potential footgun whereby a user that has
> > > been using a number without a "0x" prefix with other bcfg commands
> > > finds that, on using that exact same number with '-opt', it has this
> > > time unexpectedly been interpreted as a decimal number and they have
> > > modified
> > > (corrupted) an unrelated load option. For example, a user may have
> > > been specifying "10" to other commands to have them act on the 16th
> > > option (because simply "10", without any prefix, is how 'bcfg boot
> > > dump' displayed the option number for the 16th option). Unfortunately
> > > for them, if they also use '-opt' with "10" it would unexpectedly and
> > > inconsistently act on the 10th option.
> > >
> > > CC: Jaben Carsey
> > > CC: Ray Ni
> > > Signed-off-by: Jonathan Watt
> > > ---
> > > ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.c
> |
> > > 2
> > > +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git
> > >
> a/ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.c
> > >
> b/ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.c
> > > index d033c7c1dc59..e8b48b4990dd 100644
> > > ---
> > >
> a/ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.c
> > > +++
> > >
> b/ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.c
> > > @@ -1019,7 +1019,7 @@ BcfgAddOpt(
> > >//
> > >// Get the index of the variable we are changing.
> > >//
> > > - Status = ShellConvertStringToUint64(Walker, , FALSE,
> > > TRUE);
> > > + Status = ShellConvertStringToUint64(Walker, , TRUE,
> > > + TRUE);
> > >if (EFI_ERROR(Status) || (((UINT16)Intermediate) != Intermediate)
> > > || StrStr(Walker, L" ") == NULL || ((UINT16)Intermediate) >
> > > ((UINT16)OrderCount)) {
> > > ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN (STR_GEN_PARAM_INV),
> > > gShellBcfgHiiHandle, L"bcfg", L"Option Index");
> > > ShellStatus = SHELL_INVALID_PARAMETER;
> > > --
> > > 2.21.0
> >
> >
> >
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#40116): https://edk2.groups.io/g/devel/message/40116
Mute This Topic: https://groups.io/mt/31520134/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v1 1/1] ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' option
This patch looks good for me.
Reviewed-by: Zhichao Gao
But when I view the command in UEFI SHELL 2.2 spec:
...
bcfg driver|boot [-opt # [[filename]|["data"]] | [KeyData *]]
...
-opt
Modify the optional data associated with a driver or boot option. Followed
either by the filename of the file which contains the binary data to be
associated with the driver or boot option optional data, or else the
quote-delimited data that will be associated with the driver or boot option
optional data.
...
This description lack the comment of '#' parameter and that may make the
consumer confused. Usually consumers would regard it as the same in other
option, such as ' bcfg driver|boot [rm #]'. The '#' is clearly descripted as a
hexadecimal parameter:
rm
Remove an option. The # parameter lists the option number to remove in
hexadecimal.
So I think we should update the shell spec by the way.
Thanks,
Zhichao
> -Original Message-
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Ni,
> Ray
> Sent: Monday, May 6, 2019 10:02 PM
> To: jw...@jwatt.org; devel@edk2.groups.io
> Cc: Carsey, Jaben ; Bi, Dandan
>
> Subject: Re: [edk2-devel] [PATCH v1 1/1] ShellPkg/UefiShellBcfgCommandLib:
> Fix '-opt' option
>
> Dandan,
> Can you please help to review?
>
> Thanks,
> Ray
>
> > -Original Message-
> > From: jw...@jwatt.org [mailto:jw...@jwatt.org]
> > Sent: Monday, May 6, 2019 9:03 PM
> > To: devel@edk2.groups.io
> > Cc: Carsey, Jaben ; Ni, Ray
> > Subject: [PATCH v1 1/1] ShellPkg/UefiShellBcfgCommandLib: Fix '-opt'
> > option
> >
> > From: Jonathan Watt
> >
> > For all other bcfg commands the "#" (option number) argument(s) are
> > treated as hexedecimal values regardless of whether or not they are
> > prefixed by "0x". This change fixes '-opt' to handle its "#"
> > (option number) argument consistently with the other commands.
> >
> > Making this change removes a potential footgun whereby a user that has
> > been using a number without a "0x" prefix with other bcfg commands
> > finds that, on using that exact same number with '-opt', it has this
> > time unexpectedly been interpreted as a decimal number and they have
> > modified
> > (corrupted) an unrelated load option. For example, a user may have
> > been specifying "10" to other commands to have them act on the 16th
> > option (because simply "10", without any prefix, is how 'bcfg boot
> > dump' displayed the option number for the 16th option). Unfortunately
> > for them, if they also use '-opt' with "10" it would unexpectedly and
> > inconsistently act on the 10th option.
> >
> > CC: Jaben Carsey
> > CC: Ray Ni
> > Signed-off-by: Jonathan Watt
> > ---
> > ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.c |
> > 2
> > +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git
> > a/ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.c
> > b/ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.c
> > index d033c7c1dc59..e8b48b4990dd 100644
> > ---
> > a/ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.c
> > +++
> > b/ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.c
> > @@ -1019,7 +1019,7 @@ BcfgAddOpt(
> >//
> >// Get the index of the variable we are changing.
> >//
> > - Status = ShellConvertStringToUint64(Walker, , FALSE,
> > TRUE);
> > + Status = ShellConvertStringToUint64(Walker, , TRUE,
> > + TRUE);
> >if (EFI_ERROR(Status) || (((UINT16)Intermediate) != Intermediate)
> > || StrStr(Walker, L" ") == NULL || ((UINT16)Intermediate) >
> > ((UINT16)OrderCount)) {
> > ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN (STR_GEN_PARAM_INV),
> > gShellBcfgHiiHandle, L"bcfg", L"Option Index");
> > ShellStatus = SHELL_INVALID_PARAMETER;
> > --
> > 2.21.0
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#40099): https://edk2.groups.io/g/devel/message/40099
Mute This Topic: https://groups.io/mt/31520134/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v1 1/1] ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' option
Dandan,
Can you please help to review?
Thanks,
Ray
> -Original Message-
> From: jw...@jwatt.org [mailto:jw...@jwatt.org]
> Sent: Monday, May 6, 2019 9:03 PM
> To: devel@edk2.groups.io
> Cc: Carsey, Jaben ; Ni, Ray
> Subject: [PATCH v1 1/1] ShellPkg/UefiShellBcfgCommandLib: Fix '-opt' option
>
> From: Jonathan Watt
>
> For all other bcfg commands the "#" (option number) argument(s) are
> treated as hexedecimal values regardless of whether or not they are prefixed
> by "0x". This change fixes '-opt' to handle its "#"
> (option number) argument consistently with the other commands.
>
> Making this change removes a potential footgun whereby a user that has
> been using a number without a "0x" prefix with other bcfg commands finds
> that, on using that exact same number with '-opt', it has this time
> unexpectedly been interpreted as a decimal number and they have modified
> (corrupted) an unrelated load option. For example, a user may have been
> specifying "10" to other commands to have them act on the 16th option
> (because simply "10", without any prefix, is how 'bcfg boot dump' displayed
> the option number for the 16th option). Unfortunately for them, if they also
> use '-opt' with "10" it would unexpectedly and inconsistently act on the 10th
> option.
>
> CC: Jaben Carsey
> CC: Ray Ni
> Signed-off-by: Jonathan Watt
> ---
> ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.c | 2
> +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git
> a/ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.c
> b/ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.c
> index d033c7c1dc59..e8b48b4990dd 100644
> --- a/ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.c
> +++
> b/ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.c
> @@ -1019,7 +1019,7 @@ BcfgAddOpt(
>//
>// Get the index of the variable we are changing.
>//
> - Status = ShellConvertStringToUint64(Walker, , FALSE, TRUE);
> + Status = ShellConvertStringToUint64(Walker, , TRUE,
> + TRUE);
>if (EFI_ERROR(Status) || (((UINT16)Intermediate) != Intermediate) ||
> StrStr(Walker, L" ") == NULL || ((UINT16)Intermediate) >
> ((UINT16)OrderCount)) {
> ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN (STR_GEN_PARAM_INV),
> gShellBcfgHiiHandle, L"bcfg", L"Option Index");
> ShellStatus = SHELL_INVALID_PARAMETER;
> --
> 2.21.0
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#40041): https://edk2.groups.io/g/devel/message/40041
Mute This Topic: https://groups.io/mt/31520134/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
