Re: Commit: ARM SIM: Add tracing support.

[Doug Evans <dje at google dot com>]

On Mon, Mar 17, 2014 at 10:30 AM, Nicholas Clifton <nickc@redhat.com> wrote:
> Hi Doug,
>
>> Nick, can we tighten up the argument parsing here?
>> IIUC, -tfoo will get recognized as -t.
>
>
> Actually this does not happen.  You are correct that the code in sim_open()
> is lax in parsing the command line, but the code in
> sim_target_parse_command_line() is not.  You do get a slightly unhelpful
> error message however:
>
>   % run -tfoo a.out
>   % run: invalid option -- 'f'
>   Usage: run [options] program [program args]
>   Options:
>   [....]
>
> Which implies that you can combine command line options in a manner similar
> to tar.  You cannot however:
>
>   % run -td a.out
>   run: invalid option -- 'd'
>   [...]
>
>   % run -t -d a.out
>   pc: 8120,  movs r0, #22
>   pc: 8124,  add  r1, pc, #272    ; 0x110
>   [...]
>
> I am not sure if this is worth fixing.  I suspect that has been in the
> simulator common framework for ages.

Huh.
I took a closer look.

run.c does let one specify multiple single letter options:

  while ((i = getopt (ac, av, "a:c:m:op:s:tv")) != EOF)

but your patch adds its own special -t processing.
[And since 'd' doesn't appear in the arg to getopt that's why -td complains.]

I was going to suggest adding -t to sim_target_display_usage but I see
that is already handled by the common code.
Your patch adds new -d and -z options which is why they're added to
sim_target_display_usage.

So I guess I have more questions.
What happens if you remove all -t option parsing from your patch, and
just let the existing code handle it?
And  what happens if you remove -d and -z parsing from sim_open in
your patch, and leave it to just sim_target_parse_command_line?
Why parse them in two places? Plus, IIUC,
sim_target_parse_command_line will have removed -d/-z by the time
sim_open looks for them.

btw, note that -m processing in wrapper.c:sim_open is written the way
it is because it takes an argument (-mfoo or -m foo).
[I'm guessing cut-n-pasting from that is how the current patch ended
up processing -d/-t/-z that way in sim_open.]