| Message ID | 1430924229-20469-5-git-send-email-leon.alrae@imgtec.com |
|---|---|
| State | New |
| Headers | show |
> On 06 May 2015, at 17:57, Leon Alrae <leon.alrae@imgtec.com> wrote: > > +static int add_semihosting_arg(const char *name, const char *val, void *opaque) > +{ > + SemihostingConfig *s = opaque; > + if (strcmp(name, "arg") == 0) { > + s->argc++; > + s->argv = g_realloc(s->argv, s->argc * sizeof(void *)); > + s->argv[s->argc - 1] = val; > + } > + return 0; > +} being done at init time probably it has no impact, but, as a matter of style, I would avoid iterating realloc when the buffer size is actually known. is it that difficult to count the "arg"s and correctly alloc the array? regards, Liviu
On 07/05/2015 07:51, Liviu Ionescu wrote: > >> On 06 May 2015, at 17:57, Leon Alrae <leon.alrae@imgtec.com> wrote: >> >> +static int add_semihosting_arg(const char *name, const char *val, void *opaque) >> +{ >> + SemihostingConfig *s = opaque; >> + if (strcmp(name, "arg") == 0) { >> + s->argc++; >> + s->argv = g_realloc(s->argv, s->argc * sizeof(void *)); >> + s->argv[s->argc - 1] = val; >> + } >> + return 0; >> +} > > being done at init time probably it has no impact, but, as a matter of style, I would avoid iterating realloc when the buffer size is actually known. At this point QEMU doesn't know the buffer size, thus we need to traverse the list of sub-arguments to determine the number of args and also to get the value (i.e. string) of each arg. > > is it that difficult to count the "arg"s and correctly alloc the array? This probably would require going through the list twice which wouldn't be better in my opinion. Leon
> On 07 May 2015, at 12:52, Leon Alrae <leon.alrae@imgtec.com> wrote: > >> is it that difficult to count the "arg"s and correctly alloc the array? > > This probably would require going through the list twice the code to iterate is already there, for other options, you just need to add a branch to the existing case. regards, Liviu
On 07/05/2015 12:50, Liviu Ionescu wrote: > >> On 07 May 2015, at 12:52, Leon Alrae <leon.alrae@imgtec.com> wrote: >> >>> is it that difficult to count the "arg"s and correctly alloc the array? >> >> This probably would require going through the list twice > > the code to iterate is already there, for other options, you just need to add a branch to the existing case. Do you mean modifying QEMU option parser? But I don't see anything wrong in using qemu_opt_foreach(). Leon
> On 07 May 2015, at 15:21, Leon Alrae <leon.alrae@imgtec.com> wrote: > > On 07/05/2015 12:50, Liviu Ionescu wrote: >> >>> On 07 May 2015, at 12:52, Leon Alrae <leon.alrae@imgtec.com> wrote: >>> >>>> is it that difficult to count the "arg"s and correctly alloc the array? >>> >>> This probably would require going through the list twice >> >> the code to iterate is already there, for other options, you just need to add a branch to the existing case. > > Do you mean modifying QEMU option parser? not at all. option parsing is done anyway in two passes. the first pass is a short loop, used to identify nodeconfig & nouserconfig. the second pass is the big for(;;) loop. you can use this first pass to count the number of args, similarly as I did in my patch. regards, Liviu
On 07/05/2015 13:35, Liviu Ionescu wrote: > >> On 07 May 2015, at 15:21, Leon Alrae <leon.alrae@imgtec.com> wrote: >> >> On 07/05/2015 12:50, Liviu Ionescu wrote: >>> >>>> On 07 May 2015, at 12:52, Leon Alrae <leon.alrae@imgtec.com> wrote: >>>> >>>>> is it that difficult to count the "arg"s and correctly alloc the array? >>>> >>>> This probably would require going through the list twice >>> >>> the code to iterate is already there, for other options, you just need to add a branch to the existing case. >> >> Do you mean modifying QEMU option parser? > > not at all. > > option parsing is done anyway in two passes. the first pass is a short loop, used to identify nodeconfig & nouserconfig. the second pass is the big for(;;) loop. you can use this first pass to count the number of args, similarly as I did in my patch. > I think parsing -semihosting-config sub-options should happen inside "case QEMU_OPTION_semihosting_config", just to be consistent with other options. Leon
diff --git a/include/exec/semihost.h b/include/exec/semihost.h index c2f0bcb..6e4e8c0 100644 --- a/include/exec/semihost.h +++ b/include/exec/semihost.h @@ -36,9 +36,21 @@ static inline SemihostingTarget semihosting_get_target(void) { return SEMIHOSTING_TARGET_AUTO; } + +static inline const char *semihosting_get_arg(int i) +{ + return NULL; +} + +static inline int semihosting_get_argc(void) +{ + return 0; +} #else bool semihosting_enabled(void); SemihostingTarget semihosting_get_target(void); +const char *semihosting_get_arg(int i); +int semihosting_get_argc(void); #endif #endif diff --git a/qemu-options.hx b/qemu-options.hx index ec356f6..ed2a7e8 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -3296,14 +3296,16 @@ STEXI Enable semihosting mode (ARM, M68K, Xtensa only). ETEXI DEF("semihosting-config", HAS_ARG, QEMU_OPTION_semihosting_config, - "-semihosting-config [enable=on|off,]target=native|gdb|auto semihosting configuration\n", + "-semihosting-config [enable=on|off][,target=native|gdb|auto][,arg=str[,...]]\n" \ + " semihosting configuration\n", QEMU_ARCH_ARM | QEMU_ARCH_M68K | QEMU_ARCH_XTENSA | QEMU_ARCH_LM32) STEXI -@item -semihosting-config [enable=on|off,]target=native|gdb|auto +@item -semihosting-config [enable=on|off][,target=native|gdb|auto][,arg=str[,...]] @findex -semihosting-config Enable semihosting and define where the semihosting calls will be addressed, to QEMU (@code{native}) or to GDB (@code{gdb}). The default is @code{auto}, which means -@code{gdb} during debug sessions and @code{native} otherwise (ARM, M68K, Xtensa only). +@code{gdb} during debug sessions and @code{native} otherwise. The @var{arg} allows to +pass input arguments, can be used multiple times to build up a list (ARM, M68K, Xtensa only) ETEXI DEF("old-param", 0, QEMU_OPTION_old_param, "-old-param old param mode\n", QEMU_ARCH_ARM) diff --git a/vl.c b/vl.c index 82586c7..203ac86 100644 --- a/vl.c +++ b/vl.c @@ -486,6 +486,9 @@ static QemuOptsList qemu_semihosting_config_opts = { }, { .name = "target", .type = QEMU_OPT_STRING, + }, { + .name = "arg", + .type = QEMU_OPT_STRING, }, { /* end of list */ } }, @@ -1230,6 +1233,8 @@ static void configure_msg(QemuOpts *opts) typedef struct SemihostingConfig { bool allowed; SemihostingTarget target; + const char **argv; + int argc; } SemihostingConfig; static SemihostingConfig semihosting; @@ -1244,6 +1249,31 @@ SemihostingTarget semihosting_get_target(void) return semihosting.target; } +const char *semihosting_get_arg(int i) +{ + if (i >= semihosting.argc) { + return NULL; + } + + return semihosting.argv[i]; +} + +int semihosting_get_argc(void) +{ + return semihosting.argc; +} + +static int add_semihosting_arg(const char *name, const char *val, void *opaque) +{ + SemihostingConfig *s = opaque; + if (strcmp(name, "arg") == 0) { + s->argc++; + s->argv = g_realloc(s->argv, s->argc * sizeof(void *)); + s->argv[s->argc - 1] = val; + } + return 0; +} + /***********************************************************/ /* USB devices */ @@ -3592,6 +3622,9 @@ int main(int argc, char **argv, char **envp) } else { semihosting.target = SEMIHOSTING_TARGET_AUTO; } + /* Set semihosting argument count and vector */ + qemu_opt_foreach(opts, add_semihosting_arg, + &semihosting, 0); } else { fprintf(stderr, "Unsupported semihosting-config %s\n", optarg);
Add new "arg" sub-argument to the --semihosting-config allowing to pass multiple input argument separately. It is required for example by UHI semihosting to construct argc and argv. Signed-off-by: Leon Alrae <leon.alrae@imgtec.com> --- include/exec/semihost.h | 12 ++++++++++++ qemu-options.hx | 8 +++++--- vl.c | 33 +++++++++++++++++++++++++++++++++ 3 files changed, 50 insertions(+), 3 deletions(-)