Re: [PATCH] handle dprintf error more gracefully

[Antoine Tremblay <antoine dot tremblay at ericsson dot com>]

> Now, that seems an unfortunate change I missed before, but in any
> case, with MI async ("set mi-async on" + "maint set target-async on"),
> there's really no command associated with the error anymore,
> so ^error wouldn't be good.  So at least for async, *stopped would be
> better.  Probably with a new "error" reason or some such,
> like *stopped,reason="error".
>
> Alternatively, a new *error asynchronous MI event could be added,
> and then the frontend would be responsible for refresh all it's state
> when it got that, just like it should when it gets a synchronous ^error
> command result.  (I haven't thought this options fully through.)
>
> Note that making sure the frontend ends up with the correct thread
> state on error is already the job of finish_thread_state_cleanup
> currently.  fetch_inferior_event does install that cleanup.  However,
> finish_thread_state_cleanup does not handle *running -> *stopped,
> only *stopped -> *running...  So the fix could/should be around here.
>
> Thanks,
> Pedro Alves

I've been checking how to implement this with 
finish_thread_state_cleanup and
I ran into a few issues that I'm a bit unsure how to solve.

First problem is that finish_thread_state_cleanup gets explicitely called
by normal_stop around infrun.c:6527. So in the normal case and in the 
failure
case this cleanup gets called.

This means that if I were to use observer_notify_normal_stop(NULL, 
false); for
example in the case of *running -> *stopped in finish_thread_state, to 
handle
a possible failure we would get two *stopped events.

Maybe I could avoid calling this when I know it's a success case but
finish_thread_state_cleanup and finish_thread_sate should not be tied
to an error case. It's legitimate to call these in the normal case.
Like it is done for  observer_notify_target_resumed. This is a normal
case. If I understand correctly ?

Then I tought ok let's always call finish_thread_state_cleanup and rely 
on it
on the success case too like for resume but I need to be able to call
observer_notify_normal_stop with it's proper arguments from the normal_stop
call.

It's too bad that finish_thread_state can't handle all the states, and 
actually
this led me to believe that we should not use it in the success case for 
resume
and have a new cleanup only for the error cases or rename the function.
Since it does not really sync the front end state.

But the implications of that seem large for the corner case it handles.

So I don't think finish_thread_state_cleanup is a good place to fix the 
issue.
I think it would be better to be directly in some catch probably around
handle_signal_stop.. not sure where exactly yet...

However I think the way of doing a observer_notify_normal_stop with 
reason error
is much better then what I had done at first !! :) And so use that.

Does it sound like a plan ?

Also, for the global *error.. I'm not sure, I think it's better even if 
we give
no guarantee on the state, that we try to advertise it to the frontend 
as much
as possible even in case of error... *stopped,reason does a better job 
at that
then *error... even if *error would be more general....

Regards,

Antoine Tremblay