RE: [MI] argv/argc/args

RE: [MI] argv/argc/args

[Marc Khouzam]

Hi,

just to give input on DSF's use of these commands,
although that doesn't speak for any other frontend.

> 	-exec-next
> 	-exec-next-instruction
> 	-exec-step
> 	-exec-step-instruction
> 	-exec-continue

DSF does use a parameter on some of these but it is only an integer.
So, no problem here.

 
> The bad ones are:
> 
> 	-exec-until

We use a parameter of format file:line
I guess it could be a problem if the file name had a space in it.

> 	-target-download

Not used yet.  But any future use will require to have some special
code if we want to support older GDBs.

> 	-target-select
> 

The parameters here are host:port or serialDevice, which I believe
will not contain spaces.  Should be fine.

> 	-exec-run
 
No parameters used by DSF.

> 	-exec-return
> 

No parameters used by DSF.

> So, we have 3 commands for which requiring the input to be 
> quoted per MI rules
> will cause issues; and fixing those issues will require 
> changing other parts of
> GDB to avoid parsing filenames, which is risky at this point. 
> It appears, that
> instead of reverting my original patch, we can just path 
> those 3 commands
> via CLI directly. Does the plan sound reasonable?

For DSF, it should mean no changes, so I like this solution.

> Ideally, we'd require
> all commands to use MI quoting, but this might break lots of 
> things. You
> might recall that our non-stop delivery to you started to 
> require quoting for
> -break-condition -- which you did not exactly like :-)

Yes, I remember :-)
Backwards compatibility is always a pain.

Re: [MI] argv/argc/args

[Vladimir Prus]

On Wednesday 25 June 2008 21:21:07 Marc Khouzam wrote:
> Hi,
> 
> just to give input on DSF's use of these commands,
> although that doesn't speak for any other frontend.
> 
> > 	-exec-next
> > 	-exec-next-instruction
> > 	-exec-step
> > 	-exec-step-instruction
> > 	-exec-continue
> 
> DSF does use a parameter on some of these but it is only an integer.
> So, no problem here.
> 
>  
> > The bad ones are:
> > 
> > 	-exec-until
> 
> We use a parameter of format file:line
> I guess it could be a problem if the file name had a space in it.
> 
> > 	-target-download
> 
> Not used yet.  But any future use will require to have some special
> code if we want to support older GDBs.
> 
> > 	-target-select
> > 
> 
> The parameters here are host:port or serialDevice, which I believe
> will not contain spaces.  Should be fine.

Well, it can also be "| some-program lots-of-parameters", so quoting
is an issue. (And Sourcery G++ does use that ;-)

> > So, we have 3 commands for which requiring the input to be 
> > quoted per MI rules
> > will cause issues; and fixing those issues will require 
> > changing other parts of
> > GDB to avoid parsing filenames, which is risky at this point. 
> > It appears, that
> > instead of reverting my original patch, we can just path 
> > those 3 commands
> > via CLI directly. Does the plan sound reasonable?
> 
> For DSF, it should mean no changes, so I like this solution.

Ok.

- Volodya

Re: [MI] argv/argc/args

[Vladimir Prus]

On Friday 27 June 2008 17:53:28 Daniel Jacobowitz wrote:
> On Wed, Jun 25, 2008 at 01:06:27PM +0400, Vladimir Prus wrote:
> > So, we have 3 commands for which requiring the input to be quoted per MI rules
> > will cause issues; and fixing those issues will require changing other parts of
> > GDB to avoid parsing filenames, which is risky at this point. It appears, that
> > instead of reverting my original patch, we can just path those 3 commands
> > via CLI directly. Does the plan sound reasonable?
> 
> Can we use the same approach for -exec-run/-exec-return now that some
> of your other async-related patches are in?  I know it's less likely,
> but we can't rule out some frontend using them.

I think I can convert those, too.

> For -target-select, -target-download, and -exec-until your approach
> sounds great to me.  We do want these things to be quoted normally
> someday but I think it has to wait for mi3.

Ok, I'll see if this nice idea actually works out :-)

- Volodya

Re: [MI] argv/argc/args

[Daniel Jacobowitz]

On Wed, Jun 25, 2008 at 01:06:27PM +0400, Vladimir Prus wrote:
> So, we have 3 commands for which requiring the input to be quoted per MI rules
> will cause issues; and fixing those issues will require changing other parts of
> GDB to avoid parsing filenames, which is risky at this point. It appears, that
> instead of reverting my original patch, we can just path those 3 commands
> via CLI directly. Does the plan sound reasonable?

Can we use the same approach for -exec-run/-exec-return now that some
of your other async-related patches are in?  I know it's less likely,
but we can't rule out some frontend using them.

For -target-select, -target-download, and -exec-until your approach
sounds great to me.  We do want these things to be quoted normally
someday but I think it has to wait for mi3.

-- 
Daniel Jacobowitz
CodeSourcery

RE: [MI] argv/argc/args

[Marc Khouzam]

> From: Vladimir Prus
> Sent: Wednesday, June 25, 2008 5:06 AM
> 
> In GDB 6.8 codebase, there were 3 different ways an MI 
> command can be processed:
> 
> 1. Sneaking via CLI directly (calling execute_command)
> 2. Calling a function, that takes argc/argv pair
> 3. Calling a function, that takes a string 'args' parameter with
> the raw part of MI command string after the command name.
> 
> Some time ago, I've checked in a patch to remove (3), to clarify the
> code. Dan has raised concerns about backward compatibility, so here's
> an attempt to analyze the change afresh.

I never quite understood the quoting for MI commands.  If I recall,
some commands need quotes when they use spaces, while others are not
allowed to use quotes.  I does cause a bit of a headache.

What is the impact of removing (3) above?  Quotes will be needed
or will not be allowed?

Thanks

marc

[MI] argv/argc/args

[Vladimir Prus]

In GDB 6.8 codebase, there were 3 different ways an MI command can be processed:

1. Sneaking via CLI directly (calling execute_command)
2. Calling a function, that takes argc/argv pair
3. Calling a function, that takes a string 'args' parameter with
the raw part of MI command string after the command name.

Some time ago, I've checked in a patch to remove (3), to clarify the
code. Dan has raised concerns about backward compatibility, so here's
an attempt to analyze the change afresh.

The following is the complete list of command affected by my patch:

	-exec-run
	-exec-next
	-exec-next-instruction
	-exec-step
	-exec-step-instruction
	-exec-finish
	-exec-until
	-exec-return
	-exec-continue
	-exec-interrupt
	-target-download
	-target-select

The trivial ones are:

	-exec-finish
	-exec-interrupt

Those commands don't take any parameter at all, so no issues.

The following commands are "easy":

	-exec-next
	-exec-next-instruction
	-exec-step
	-exec-step-instruction
	-exec-continue

They are not documented to take any parameters. In practice, they will
accept a parameter -- telling either the number of steps to perform,
or the number of times to ignore a breakpoint we're stopped at. We'll have
a problem here only if frontend indeed passes extra parameter, and that
parameter is expression (not a simple number) and that expression has
spaces. Seems to me this is unlikely.


The bad ones are:

	-exec-until
	-target-download
	-target-select

Those are documented to accept a parameter, and, furthermore, this parameter may
contain spaces. So, if we strip quoting at MI level, we pass unquoted string futher.
For example, -exec-until eventually calls until_break_command, which calls 
decode_line_1, and if we pass a filename with spaces to -exec-until and quotes
are stripped before calling until_break_command, this will not work. The other
two commands are even more risky.

The gray ones are:

	-exec-run

    this is not documented to take a parameter but may. I think, though, that
    frontends probably use -exec-arguments, instead of relying on this undocumented
    behaviour.

	-exec-return

    This one is not documented as returning an expression -- this is probably a
    documentation bug. Frontend passing an argument to this command will be broken
    if it tries to pass an unquoted expression with spaces, which is probably
    not common.

So, we have 3 commands for which requiring the input to be quoted per MI rules
will cause issues; and fixing those issues will require changing other parts of
GDB to avoid parsing filenames, which is risky at this point. It appears, that
instead of reverting my original patch, we can just path those 3 commands
via CLI directly. Does the plan sound reasonable?

- Volodya