| Message ID | 1518082525-5995-1-git-send-email-alex.kiernan@gmail.com |
|---|---|
| State | Superseded |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | [U-Boot] Fix --noheader on fw_printenv | expand |
On 08.02.2018 10:35, Alex Kiernan wrote: > Using fw_printenv with --noheader fails: > > root@nrr-922:~# fw_printenv --noheader arch > ## Error: `-n' option requires exactly one argument I think it would work with --noheader=arch > > Whereas -n works: > > root@nrr-922:~# fw_printenv -n arch > arm > > The single argument it's expecting isn't taken from getopt parsing, > but instead from the remaining argv arguments. That makes sense. But the commit log text above is kind of unrelated/not relevant, I would just use this two lines as git message. -- Stefan > > Signed-off-by: Alex Kiernan <alex.kiernan@gmail.com> > --- > > tools/env/fw_env_main.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tools/env/fw_env_main.c b/tools/env/fw_env_main.c > index 6fdf41c..d93a915 100644 > --- a/tools/env/fw_env_main.c > +++ b/tools/env/fw_env_main.c > @@ -46,7 +46,7 @@ static struct option long_options[] = { > {"config", required_argument, NULL, 'c'}, > {"help", no_argument, NULL, 'h'}, > {"script", required_argument, NULL, 's'}, > - {"noheader", required_argument, NULL, 'n'}, > + {"noheader", no_argument, NULL, 'n'}, > {"lock", required_argument, NULL, 'l'}, > {"version", no_argument, NULL, 'v'}, > {NULL, 0, NULL, 0}
On Thu, Feb 8, 2018 at 3:37 PM, <stefan@agner.ch> wrote: > On 08.02.2018 10:35, Alex Kiernan wrote: >> Using fw_printenv with --noheader fails: >> >> root@nrr-922:~# fw_printenv --noheader arch >> ## Error: `-n' option requires exactly one argument > > I think it would work with --noheader=arch > It doesn't: root@nrr-922:~# fw_printenv --noheader=arch ## Error: `-n' option requires exactly one argument Probably I should fix the error too as it's misleading. >> >> Whereas -n works: >> >> root@nrr-922:~# fw_printenv -n arch >> arm >> >> The single argument it's expecting isn't taken from getopt parsing, >> but instead from the remaining argv arguments. > > That makes sense. But the commit log text above is kind of unrelated/not > relevant, I would just use this two lines as git message. > Happy to chop it down to the last two lines though as it does include the relevant details.
On 08.02.2018 17:17, Alex Kiernan wrote: > On Thu, Feb 8, 2018 at 3:37 PM, <stefan@agner.ch> wrote: >> On 08.02.2018 10:35, Alex Kiernan wrote: >>> Using fw_printenv with --noheader fails: >>> >>> root@nrr-922:~# fw_printenv --noheader arch >>> ## Error: `-n' option requires exactly one argument >> >> I think it would work with --noheader=arch >> > > It doesn't: > > root@nrr-922:~# fw_printenv --noheader=arch > ## Error: `-n' option requires exactly one argument > > Probably I should fix the error too as it's misleading. > This comes from getopt_long, so I don't think you can fix it. The getopt string in parse_printenv_args actually says argument "n" has no argument (no colon), so it actually is surprising that it prints that... Maybe this is from the getopt_long call in parse_common_args? >>> >>> Whereas -n works: >>> >>> root@nrr-922:~# fw_printenv -n arch >>> arm >>> >>> The single argument it's expecting isn't taken from getopt parsing, >>> but instead from the remaining argv arguments. >> >> That makes sense. But the commit log text above is kind of unrelated/not >> relevant, I would just use this two lines as git message. >> > > Happy to chop it down to the last two lines though as it does include > the relevant details. IMHO the error is that struct option long_options ask for an argument whereas the optstring passed to getopt_long doesn't. The rather erratic is not really relevant and should not be mentioned. -- Stefan
On Thu, Feb 8, 2018 at 4:40 PM, Stefan Agner <stefan@agner.ch> wrote: > On 08.02.2018 17:17, Alex Kiernan wrote: >> On Thu, Feb 8, 2018 at 3:37 PM, <stefan@agner.ch> wrote: >>> On 08.02.2018 10:35, Alex Kiernan wrote: >>>> Using fw_printenv with --noheader fails: >>>> >>>> root@nrr-922:~# fw_printenv --noheader arch >>>> ## Error: `-n' option requires exactly one argument >>> >>> I think it would work with --noheader=arch >>> >> >> It doesn't: >> >> root@nrr-922:~# fw_printenv --noheader=arch >> ## Error: `-n' option requires exactly one argument >> >> Probably I should fix the error too as it's misleading. >> > > This comes from getopt_long, so I don't think you can fix it. The getopt > string in parse_printenv_args actually says argument "n" has no argument > (no colon), so it actually is surprising that it prints that... > > Maybe this is from the getopt_long call in parse_common_args? > It's in fw_env.c: 449 int fw_printenv(int argc, char *argv[], int value_only, struct env_opts *opts) 450 { 451 int i, rc = 0; 452 453 if (value_only && argc != 1) { 454 fprintf(stderr, 455 "## Error: `-n' option requires exactly one argument\n"); 456 return -1; 457 } 458 You pass it one argument, getopt_long consumes it, the call to getopt_long does nothing with the that argument and then this check fails because there are no args left. Though you've made me realise it does of course work if you call it like this! root@nrr-922:~# fw_printenv --noheader=foo arch arm I guess you could rewrite it so it's actually an optarg, not picked out of argv, but you'd still want to check that there's no more args, so really not sure you gain very much.
diff --git a/tools/env/fw_env_main.c b/tools/env/fw_env_main.c index 6fdf41c..d93a915 100644 --- a/tools/env/fw_env_main.c +++ b/tools/env/fw_env_main.c @@ -46,7 +46,7 @@ static struct option long_options[] = { {"config", required_argument, NULL, 'c'}, {"help", no_argument, NULL, 'h'}, {"script", required_argument, NULL, 's'}, - {"noheader", required_argument, NULL, 'n'}, + {"noheader", no_argument, NULL, 'n'}, {"lock", required_argument, NULL, 'l'}, {"version", no_argument, NULL, 'v'}, {NULL, 0, NULL, 0}
Using fw_printenv with --noheader fails: root@nrr-922:~# fw_printenv --noheader arch ## Error: `-n' option requires exactly one argument Whereas -n works: root@nrr-922:~# fw_printenv -n arch arm The single argument it's expecting isn't taken from getopt parsing, but instead from the remaining argv arguments. Signed-off-by: Alex Kiernan <alex.kiernan@gmail.com> --- tools/env/fw_env_main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)