[RFA] JMTI Exception Events

[Tom Tromey]

>>>>> "Kyle" == Kyle Galloway <kgallowa@redhat.com> writes:

Kyle> Before I commit this, I wanted to clear up which files I should
Kyle> commit. Do I need to include more than sources.am and Makefile.am?
Kyle> For example, should the changed Makefile.ins go in as well?

Yes, commit everything that is changed.

Kyle> Questions/comments/concerns?

A few minor things, but one or two substantive ones as well.  Read on..

Kyle> +// Method to check if an exception is hadled at some location (pc) in a method

Typo: "handled".

Kyle> +// (meth).  It then sets the pc to the start of the handler
Kyle> +int
Kyle> +_Jv_InterpMethod::check_handler (pc_t *pc, _Jv_InterpMethod *meth,
Kyle> +                                java::lang::Throwable *ex)

This should be declared as returning bool or jboolean, not int.
The meaning of the return value should be documented in the comment.

Kyle> +return false;

Indentation is incorrect.

Kyle> +  private static WeakHashMap<Thread, Throwable> _ex_map;

I think you probably want to initialize this here, rather than in the
ExceptionEvent constructor.  Doing it in the constructor means that
there is a race.

Static fields are initialized when the class is initialized -- so it
is already done lazily.

Kyle> +      _ex_map = new WeakHashMap ();

... when you do this I think it is preferable to write:

    .. = new WeakHashMap<Thread, Throwable> ();

Kyle> +        if (!(_ex_map.get (thr).equals (ex)))

Without an equals() method in this class, I doubt this does what you
intend.  As it is this equals() will always return false, because the
default equals() tests object identity, and a new object is never
identical to any existing one.

Kyle> +            send_event ();

It is a bit odd to have a side-effect-ing constructor.  I'd say it is
a bit more idiomatic to have a static method to dispatch the event,
which creates the event object as needed.  Seeing a raw 'new ...' as a
statement is kind of strange.

Kyle> +// This method looks up the interpreted call stack to see if the excetion will

Typo: "exception".

Tom