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

[Andrew Cagney <ac131313@redhat.com>]

> 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;
> }