This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [MI non-stop 04/11] Implement --thread and --frame.
On Saturday 28 June 2008 22:16:06 Eli Zaretskii wrote:
> > From: Vladimir Prus <vladimir@codesourcery.com>
> > Date: Sat, 28 Jun 2008 21:34:57 +0400
> >
> > Eli Zaretskii wrote:
> >
> > >> From: Vladimir Prus <vladimir@codesourcery.com>
> > >> Date: Sat, 28 Jun 2008 20:44:14 +0400
> > >>
> > >>
> > >> + if (parse->frame != -1 && !parse->thread == -1)
> > >> + error ("Cannot specify --frame without --thread");
> > >
> > > Why is this error message not in _(), while all the rest are?
> > >
> > > Btw, do we at all want error messages issued by MI commands
> > > translated?
> >
> > I think we don't, but I'm not sure.
> >
> >
> > > MI commands are invoked by a program, so error messages
> > > we generate should be understandable by a program, which probably
> > > means they should not be translated.
> >
> > It's a bit questionable. For example, the error you mention above
> > is clearly a bug in frontend. Presenting a translated version of
> > that message to the user is essentially pointless. On the other
> > hand, "Thread is running", or "Memory not accessible" messages
> > can be helpful for users. Do we need two error messages, maybe?
>
> Maybe. Do front ends show the error messages to the user, or do they
> act on them themselves (or both)?
Both, I think. I'd expect that right now, showing the message to the user
is more common than handling it internally -- because it's fairly hard to
guess, from an error message, what is this. For example, KDevelop 3.5, when
operating with pre-6.8 gdb, will handle all errors from -break-insert command
-- assuming we get an error because the location is in not-yet-loaded shared
library. But, this has the potential to hide real error -- like line number
that is out of range.
It seems like the only solution is to have some set of error codes, which
are never shown to the user, and then have the message string i18ned.
> > >> + if (strncmp (chp, "--thread", 8) == 0)
> > >
> > > Please, let's not use literal constants in this context, let's use
> > > sizeof instead.
> >
> > sizeof? For all I know, sizeof("--thread") will be wrong here.
>
> Why would it be wrong?
It will give you 9, here.
> > >> + parse->frame = strtol (chp, &chp, 10);
> > >
> > > Do we really want to disallow non-decimal numbers here? What about
> > > hex frame numbers?
> >
> > Why would frontend want to specify frame level in hex?
>
> I dunno, does the MI spec mandate decimal here? If it does, then my
> comment is hereby withdrawn.
The spec does not mandate decimal, or allow hex -- yet. Other commands that
take numbers are also silent, however the code appears to use decimal. For
example, -stack-list-args and friends use atoi -- which is 10-base.
- Volodya