Message ID | 1369835434-27727-5-git-send-email-thomas@codesourcery.com |
---|---|
State | New |
Headers | show |
On 29.05.2013, at 15:50, Thomas Schwinge wrote: > Revert the change in behavior that had been introducecd in commit > fc9c54124d134dbd76338a92a91804dab2df8166 for the -E and -U command-line > options, but keep the comma-splitting for the QEMU_SET_ENV and QEMU_UNSET_ENV > environment variables. > > Signed-off-by: Thomas Schwinge <thomas@codesourcery.com> > --- > linux-user/main.c | 51 +++++++++++++++++++++++++++++++++++---------------- > 1 file changed, 35 insertions(+), 16 deletions(-) > > diff --git linux-user/main.c linux-user/main.c > index b7d49f4..874791b 100644 > --- linux-user/main.c > +++ linux-user/main.c > @@ -3209,15 +3209,37 @@ static void handle_arg_log_filename(arg_origin whence, const char *arg) > > static void handle_arg_set_env(arg_origin whence, const char *arg) > { > - if (envlist_parse_set(envlist, arg) != 0) { > - usage(); > + switch (whence) { > + case ARG_ORIGIN_ENV: > + if (envlist_parse_set(envlist, arg) != 0) { > + usage(); > + } > + break; > + case ARG_ORIGIN_CMDLINE: > + if (envlist_setenv(envlist, arg) != 0) { > + usage(); > + } > + break; > + default: > + abort(); > } > } > > static void handle_arg_unset_env(arg_origin whence, const char *arg) > { > - if (envlist_parse_unset(envlist, arg) != 0) { > - usage(); > + switch (whence) { > + case ARG_ORIGIN_ENV: > + if (envlist_parse_unset(envlist, arg) != 0) { > + usage(); > + } > + break; > + case ARG_ORIGIN_CMDLINE: > + if (envlist_unsetenv(envlist, arg) != 0) { > + usage(); > + } > + break; > + default: > + abort(); > } > } > > @@ -3443,18 +3465,15 @@ static void usage(void) > guest_stack_size); > > printf("\n" > - "You can use -E and -U options or the QEMU_SET_ENV and\n" > - "QEMU_UNSET_ENV environment variables to set and unset\n" > - "environment variables for the target process.\n" > - "It is possible to provide several variables by separating them\n" > - "by commas in getsubopt(3) style. Additionally it is possible to\n" > - "provide the -E and -U options multiple times.\n" > - "The following lines are equivalent:\n" > - " -E var1=val2 -E var2=val2 -U LD_PRELOAD -U LD_DEBUG\n" > - " -E var1=val2,var2=val2 -U LD_PRELOAD,LD_DEBUG\n" > - " QEMU_SET_ENV=var1=val2,var2=val2 QEMU_UNSET_ENV=LD_PRELOAD,LD_DEBUG\n" > - "Note that if you provide several changes to a single variable\n" > - "the last change will stay in effect.\n"); > +"The -E and -U command-line options can be provided multiple times to set\n" > +"and unset environment variables for the target process, and -E can be used\n" > +"to specify values containing commas. When using the QEMU_SET_ENV and\n" > +"QEMU_UNSET_ENV environment variables, a comma is used in getsubopt(3) style\n" > +"to set or unset several variables. The following lines are equivalent:\n" > +" -E var1=val2 -E var2=val2 -U LD_PRELOAD -U LD_DEBUG\n" > +" QEMU_SET_ENV=var1=val2,var2=val2 QEMU_UNSET_ENV=LD_PRELOAD,LD_DEBUG\n" > +"Note that if you provide several changes to a single variable, the last\n" > +"change will stay in effect.\n"); I suppose the additional indent was on purpose. It would also be nice to document the availability of ,, here now - I doubt everyone knows that it's possible to use it here. Otherwise the patch set looks fine to me. Alex
On 29 May 2013 14:50, Thomas Schwinge <thomas@codesourcery.com> wrote: > Revert the change in behavior that had been introducecd in commit > fc9c54124d134dbd76338a92a91804dab2df8166 for the -E and -U command-line > options, but keep the comma-splitting for the QEMU_SET_ENV and QEMU_UNSET_ENV > environment variables. > > Signed-off-by: Thomas Schwinge <thomas@codesourcery.com> > --- > linux-user/main.c | 51 +++++++++++++++++++++++++++++++++++---------------- > 1 file changed, 35 insertions(+), 16 deletions(-) > > diff --git linux-user/main.c linux-user/main.c > index b7d49f4..874791b 100644 > --- linux-user/main.c > +++ linux-user/main.c > @@ -3209,15 +3209,37 @@ static void handle_arg_log_filename(arg_origin whence, const char *arg) > > static void handle_arg_set_env(arg_origin whence, const char *arg) > { > - if (envlist_parse_set(envlist, arg) != 0) { > - usage(); > + switch (whence) { > + case ARG_ORIGIN_ENV: > + if (envlist_parse_set(envlist, arg) != 0) { > + usage(); > + } > + break; > + case ARG_ORIGIN_CMDLINE: > + if (envlist_setenv(envlist, arg) != 0) { > + usage(); > + } > + break; > + default: > + abort(); > } > } I agree with Alex's comments; also this function could use a brief comment explaining why we treat env and command line differently, as a guard against somebody at a future date undoing this change in the name of simplification and conistency. thanks -- PMM
diff --git linux-user/main.c linux-user/main.c index b7d49f4..874791b 100644 --- linux-user/main.c +++ linux-user/main.c @@ -3209,15 +3209,37 @@ static void handle_arg_log_filename(arg_origin whence, const char *arg) static void handle_arg_set_env(arg_origin whence, const char *arg) { - if (envlist_parse_set(envlist, arg) != 0) { - usage(); + switch (whence) { + case ARG_ORIGIN_ENV: + if (envlist_parse_set(envlist, arg) != 0) { + usage(); + } + break; + case ARG_ORIGIN_CMDLINE: + if (envlist_setenv(envlist, arg) != 0) { + usage(); + } + break; + default: + abort(); } } static void handle_arg_unset_env(arg_origin whence, const char *arg) { - if (envlist_parse_unset(envlist, arg) != 0) { - usage(); + switch (whence) { + case ARG_ORIGIN_ENV: + if (envlist_parse_unset(envlist, arg) != 0) { + usage(); + } + break; + case ARG_ORIGIN_CMDLINE: + if (envlist_unsetenv(envlist, arg) != 0) { + usage(); + } + break; + default: + abort(); } } @@ -3443,18 +3465,15 @@ static void usage(void) guest_stack_size); printf("\n" - "You can use -E and -U options or the QEMU_SET_ENV and\n" - "QEMU_UNSET_ENV environment variables to set and unset\n" - "environment variables for the target process.\n" - "It is possible to provide several variables by separating them\n" - "by commas in getsubopt(3) style. Additionally it is possible to\n" - "provide the -E and -U options multiple times.\n" - "The following lines are equivalent:\n" - " -E var1=val2 -E var2=val2 -U LD_PRELOAD -U LD_DEBUG\n" - " -E var1=val2,var2=val2 -U LD_PRELOAD,LD_DEBUG\n" - " QEMU_SET_ENV=var1=val2,var2=val2 QEMU_UNSET_ENV=LD_PRELOAD,LD_DEBUG\n" - "Note that if you provide several changes to a single variable\n" - "the last change will stay in effect.\n"); +"The -E and -U command-line options can be provided multiple times to set\n" +"and unset environment variables for the target process, and -E can be used\n" +"to specify values containing commas. When using the QEMU_SET_ENV and\n" +"QEMU_UNSET_ENV environment variables, a comma is used in getsubopt(3) style\n" +"to set or unset several variables. The following lines are equivalent:\n" +" -E var1=val2 -E var2=val2 -U LD_PRELOAD -U LD_DEBUG\n" +" QEMU_SET_ENV=var1=val2,var2=val2 QEMU_UNSET_ENV=LD_PRELOAD,LD_DEBUG\n" +"Note that if you provide several changes to a single variable, the last\n" +"change will stay in effect.\n"); exit(1); }
Revert the change in behavior that had been introducecd in commit fc9c54124d134dbd76338a92a91804dab2df8166 for the -E and -U command-line options, but keep the comma-splitting for the QEMU_SET_ENV and QEMU_UNSET_ENV environment variables. Signed-off-by: Thomas Schwinge <thomas@codesourcery.com> --- linux-user/main.c | 51 +++++++++++++++++++++++++++++++++++---------------- 1 file changed, 35 insertions(+), 16 deletions(-)