Re: [RFA] Reverse Debugging, 2/5

[Michael Snyder <msnyder@vmware.com>]

Pedro Alves wrote:
> On Wednesday 01 October 2008 20:17:54, Michael Snyder wrote:
>> +  /* TODO: check target for capability.  */
> 
> Can we address this?  If you want to be able to query for support,
> it would be a matter of defining a new qSupported feature.

OK -- but what about existing targets that support reverse,
but don't know about the qSupported query?

When I put that comment in, I probably intended an implied
question-mark -- that is, I wasn't asserting that a query
would be useful, just wondering aloud...


> On Wednesday 01 October 2008 20:17:54, Michael Snyder wrote:
>> -         status->kind = TARGET_WAITKIND_STOPPED;
>> -         status->value.sig = TARGET_SIGNAL_0;
>> +         if (buf[1] == '0' && buf[2] == '6')
>> +           {
>> +             status->kind = TARGET_WAITKIND_NO_HISTORY;
>> +           }
>> +         else
>> +           {
>> +             status->kind = TARGET_WAITKIND_STOPPED;
>> +             status->value.sig = TARGET_SIGNAL_0;
>> +           }
> 
> This isn't really an error, it's a defined reply, so it
> looks a bit strange to me to be using an error number.
> 
> Is there a reason this can't be reported with a T stop reply and
> a special "register", like "library" -> TARGET_WAITKIND_LOADED is?
> 
> AFAICT, nothing else in the remote implementation relies
> on defined error numbers currently --- annoying at times, but
> doesn't seem to apply here.

Yeah, I hear ya -- I'm not crazy about it either, and I
don't think I knew about the idea of adding new tags onto
the "T" packet two years ago.

But... the discussion about the remote protocol for this
happened back in '06.  There are now targets out in the field
that implement it this way.  It would be bad form to break them...

We could add a new T packet tag, and then threaten to
deprecate support for the error reply at some future time.


> +
> +static enum exec_direction_kind remote_get_execdir (void)
> 
> Function name on the first column please.
> 
> +{
> +  if (remote_debug && info_verbose)
> +    printf_filtered ("remote execdir is %s\n",
> +                    remote_execdir == EXEC_FORWARD ? "forward" :
> +                    remote_execdir == EXEC_REVERSE ? "reverse" :
> +                    "unknown");
> +  return remote_execdir;
> +}
> 
> This should be made i18n aware.
> 
> Similarly in remote_set_execdir.

You got it.  ;-)

> No new vCont packets -> no plans on reverse + multi-threading ?  :-)

Noooo... it means no story *now* about reverse and multi-threading.
Can always be added in later...