Re: Re-initializing a list after the control returns to gdb...

Andrew Cagney
2003-02-27T18:44:30Z

> Attached is a prototype that appears to be working. I assumed that you
> had in mind an approach were the exported interface is nice and clean
> and the implementation uses an underlying generic mechanism with
> nasty casts and marshalling/unmarshalling.
>
> While looking more closely at gdb-events, I understood better your
> comment about doco and gdbarch.sh. I think that with the current
> prototype, it should be relatively easy to generate observer.[hc].

Yes.

gdbarch.[ch] and gdb-events.[ch] both started out as normal files. It
is only when they grew beyond a certain point that they turned into
scripts. In terms of getting it right, mumble something about `third
time lucky' :-)

> There was another approach that came to my mind when I realized that
> observer.[hc] could be generated by a script: instead of using a
> internal generic observer, we could simply duplicate the code for
> each external observer... Although it does avoid the type casting et al,
> I discarded this approach as a bit too brutal and not very elegant.

It's an internal implementation detail. Provided the code is
`interface' and `coding standard' compliant (doing no externally visible
evil, indented using gdb_indent.sh, gets past gdb_ari.sh -Wari) can any
one really complain about how version 1.0 is implemented using wire and
bits of string :-)

> I tested this code by using a little test program. I will try not to
> forget to attach it. Would there be any way to build this program or
> some equivalent somewhere in our testsuite? My (very limited) first test
> was also inside GDB itself, where I added a ada-tasks as an observer for
> the normal_stop notification, and added a call to
> observer_notify_normal_stop in normal_stop.

The testsuite/gdb.gdb directory was created for just this purpose!

> About the struct observer: the data field is currently used to hold the
> pointer to the real observer notification function. So there is no
> deallocation issue for now. I tried to make it clear that no
> deallocation mechanism has been put in place for this field by adding a
> comment next to the field declaration. It can later be extended to also
> hold some context data, if needed. However, if/when this happens, we'll
> have to extend the attach routine to accept 2 new parameters: a pointer
> to the context data, a pointer to a function to deallocate this data.
>
> I also have to admit upfront that choosing names has always been one
> of my big weaknesses. I warmly welcome suggestions.

Ditto. That is why I suggested stealing terms from the patterns book -
you get to blame the decision on someone else.

> One last thing, I omitted the copyright headers for now. I'll make sure
> to add them when I submit the RFA.
>
>
>> (oh, unless you've an immediate need for that data/context parameter,
>> i'd leave it out - such a mechanis can be left to the person that
>> actually needs)
>
>
> Cools, that saves a bit of complexity for later, although there isn't
> much more needed.
>
>
>> (shh, doco - but I have a cunning plan - see my recent post about
>> revamping gdbarch.sh into doc/gdbarch.texi).
>
>
> Right, I will add the proper documentation too. Ideally, having the doco
> in an observer.sh script would be great.

I was thinking of something like:

Chapter Observing changes in @value{GDBN} internals
Light highlevel commentary largely refering the reader to the patterns book

Appendix `observer' (observer.texi)
@defun void normal_stop (void)
Normal stop notification.
@end defun

observer.sh could then munge that into your file.

Can you please add the missing (C) notice and then commit this part
(adding it to the build & makefile).

Then slap a summary of our `grand plans' somewhere in the header.

Andrew

> #ifndef OBSERVER_H
> #define OBSERVER_H
>
> struct observer;
>
> /* normal_stop notifications. */
> typedef void (observer_normal_stop_ftype) (void);
>
> extern struct observer *
> observer_attach_normal_stop (observer_normal_stop_ftype *f);
> extern void observer_detach_normal_stop (struct observer *observer);
> extern void observer_notify_normal_stop (void);
>
> #endif /* OBSERVER_H */
>
>
>
> #include "defs.h"
> #include "observer.h"
>
> typedef void (generic_observer_notification_ftype) (const void *data,
> const void *args);
>
> struct observer
> {
> generic_observer_notification_ftype *notify;
> void *data; /* No memory management needed for this field for now. */
> };
>
> struct observer_list
> {
> struct observer_list *next;
> struct observer *observer;
> };
>
> static struct observer_list *
> xalloc_observer_list_node (void)
> {
> struct observer_list *node = XMALLOC (struct observer_list);
> node->observer = XMALLOC (struct observer);
> return node;
> }
>
> static void
> xfree_observer_list_node (struct observer_list *node)
> {
> xfree (node->observer);
> xfree (node);
> }
>
> static struct observer *
> generic_observer_attach (struct observer_list **list,
> generic_observer_notification_ftype *notify,
> void *data)
> {
> struct observer_list *observer_list = xalloc_observer_list_node ();
>
> observer_list->next = *list;
> observer_list->observer->notify = notify;
> observer_list->observer->data = data;
> *list = observer_list;
>
> return observer_list->observer;
> }
>
> static void
> generic_observer_detach (struct observer_list **list,
> const struct observer *observer)
> {
> struct observer_list *previous_node = NULL;
> struct observer_list *current_node = *list;
>
> while (current_node != NULL)
> {
> if (current_node->observer == observer)
> {
> if (previous_node != NULL)
> previous_node->next = current_node->next;
> else
> *list = current_node->next;
> xfree_observer_list_node (current_node);
> return;
> }
> previous_node = current_node;
> current_node = current_node->next;
> }
>
> /* We should never reach this point. However, this should not be
> a very serious error, so simply report a warning to the user. */
> warning ("Failed to detach observer");
> }
>
> static void
> generic_observer_notify (struct observer_list *list, const void *args)
> {
> struct observer_list *current_node = list;
>
> while (current_node != NULL)
> {
> (*current_node->observer->notify) (current_node->observer->data, args);
> current_node = current_node->next;
> }
> }
>
> /* normal_stop notifications. */
>
> static struct observer_list *normal_stop_observers = NULL;
>
> void
> observer_normal_stop_notification_stub (const void *data,
> const void *unused_args)
> {
> observer_normal_stop_ftype *notify = (observer_normal_stop_ftype *) data;
> (*notify) ();
> }
>
> struct observer *
> observer_attach_normal_stop (observer_normal_stop_ftype *f)
> {
> return generic_observer_attach (&normal_stop_observers,
> &observer_normal_stop_notification_stub,
> (void *) f);
> }
>
> void
> observer_detach_normal_stop (struct observer *observer)
> {
> generic_observer_detach (&normal_stop_observers, observer);
> }
>
> void
> observer_notify_normal_stop (void)
> {
> generic_observer_notify (normal_stop_observers, NULL);
> }
>
>
>
> #include <stdio.h>
>
> #include "observer.h"
>
> #include <stdlib.h>
> void *xmalloc (size_t size) { return malloc (size); }
> void xfree (void *ptr) { free (ptr); }
> void warning (const char *str, ...) { printf ("warning: %s.\n", str); }
>
> int first_observer = 0;
> int second_observer = 0;
> int third_observer = 0;
>
> void
> reset_counters (void)
> {
> first_observer = 0;
> second_observer = 0;
> third_observer = 0;
> }
>
> void
> check_counters (int first, int second, int third)
> {
> if (first_observer != first)
> printf ("ERROR: first observer incorrect count: %d (expected %d).\n",
> first_observer, first);
> if (second_observer != second)
> printf ("ERROR: second observer incorrect count: %d (expected %d).\n",
> second_observer, second);
> if (third_observer != third)
> printf ("ERROR: third observer incorrect count: %d (expected %d).\n",
> third_observer, third);
> }
>
> void
> test_notifications (int first, int second, int third, char *msg)
> {
> printf ("-- Sending notification (%s)...\n", msg);
> reset_counters ();
> observer_notify_normal_stop ();
> check_counters (first, second, third);
> }
>
> void
> first_observer_notification (void)
> {
> first_observer++;
> }
>
> void
> second_observer_notification (void)
> {
> second_observer++;
> }
>
> void
> third_observer_notification (void)
> {
> third_observer++;
> }
>
> void
> general_observer_test (void)
> {
> struct observer *first_obs = NULL;
> struct observer *second_obs = NULL;
> struct observer *third_obs = NULL;
>
> /* First, try sending a notification without any observers attached. */
> test_notifications (0, 0, 0, "no observer");
>
> /* Now, attach one observer, and send a notification. */
> second_obs = observer_attach_normal_stop (&second_observer_notification);
> test_notifications (0, 1, 0, "one observer");
>
> /* Remove the observer, and send a notification. */
> observer_detach_normal_stop (second_obs);
> test_notifications (0, 0, 0, "no observer");
>
> /* With a new observer. */
> first_obs = observer_attach_normal_stop (&first_observer_notification);
> test_notifications (1, 0, 0, "a new observer");
>
> /* With 2 observers. */
> second_obs = observer_attach_normal_stop (&second_observer_notification);
> test_notifications (1, 1, 0, "2 observers");
>
> /* With 3 observers. */
> third_obs = observer_attach_normal_stop (&third_observer_notification);
> test_notifications (1, 1, 1, "3 observers");
>
> /* Remove the middle observer. */
> observer_detach_normal_stop (second_obs);
> test_notifications (1, 0, 1, "middle observer removed");
>
> /* Remove first observer. */
> observer_detach_normal_stop (first_obs);
> test_notifications (0, 0, 1, "first observer removed");
>
> /* Remove last observer. */
> observer_detach_normal_stop (third_obs);
> test_notifications (0, 0, 0, "last observer removed");
>
> /* Go back to 3 observers, and remove them in a different order... */
> first_obs = observer_attach_normal_stop (&first_observer_notification);
> second_obs = observer_attach_normal_stop (&second_observer_notification);
> third_obs = observer_attach_normal_stop (&third_observer_notification);
> test_notifications (1, 1, 1, "3 observers");
>
> /* Remove the third observer... */
> observer_detach_normal_stop (third_obs);
> test_notifications (1, 1, 0, "third observer removed");
>
> /* Remove the second observer... */
> observer_detach_normal_stop (second_obs);
> test_notifications (1, 0, 0, "second observer removed");
>
> /* Remove the first observer... No more observers. */
> observer_detach_normal_stop (first_obs);
> test_notifications (0, 0, 0, "first observer removed - no more observers");
>
> }
>
> int
> main (void)
> {
> general_observer_test ();
> return 0;
> }