]> git.ipfire.org Git - thirdparty/asterisk.git/commitdiff
Merged revisions 258632 via svnmerge from
authorRussell Bryant <russell@russellbryant.com>
Thu, 22 Apr 2010 21:58:06 +0000 (21:58 +0000)
committerRussell Bryant <russell@russellbryant.com>
Thu, 22 Apr 2010 21:58:06 +0000 (21:58 +0000)
https://origsvn.digium.com/svn/asterisk/trunk

For 1.6.2, only merge the bug fixes, not the unit test.

........
  r258632 | russell | 2010-04-22 16:06:53 -0500 (Thu, 22 Apr 2010) | 22 lines

  Add ast_event subscription unit test and fix some ast_event API bugs.

  This patch introduces another test in test_event.c that exercises most of the
  subscription related ast_event API calls.  I made some minor additions to the
  existing event allocation test to increase API coverage by the test code.
  Finally, I made a list in a comment of API calls not yet touched by the test
  module as a to-do list for future test development.

  During the development of this test code, I discovered a number of bugs in
  the event API.

  1) subscriptions to AST_EVENT_ALL were not handled appropriately in a couple
     of different places.  The API allows a subscription to all event types,
     but with IE parameters, just as if it was a subscription to a specific
     event type.  However, the parameters were being ignored.  This affected
     ast_event_check_subscriber() and event distribution to subscribers.

  2) Some of the logic in ast_event_check_subscriber() for checking subscriptions
     against query parameters was wrong.

  Review: https://reviewboard.asterisk.org/r/617/
........

git-svn-id: https://origsvn.digium.com/svn/asterisk/branches/1.6.2@258672 65c4cc65-6c06-0410-ace0-fbb531ad65f3

main/event.c

index dd3cff1f7755e25894ad27dee778b2bc446c2cf8..60dde3bb284efc29556a749700c16323a6430529 100644 (file)
@@ -319,14 +319,62 @@ static void ast_event_ie_val_destroy(struct ast_event_ie_val *ie_val)
        ast_free(ie_val);
 }
 
+/*!
+ * \internal
+ * \brief Check if an ie_val matches a subscription
+ *
+ * \param sub subscription to check against
+ * \param ie_val IE value to check
+ *
+ * \retval 0 not matched
+ * \retval non-zero matched
+ */
+static int match_ie_val_to_sub(const struct ast_event_sub *sub, const struct ast_event_ie_val *ie_val)
+{
+       const struct ast_event_ie_val *sub_ie_val;
+       int res = 1;
+
+       AST_LIST_TRAVERSE(&sub->ie_vals, sub_ie_val, entry) {
+               if (sub_ie_val->ie_type == ie_val->ie_type) {
+                       break;
+               }
+       }
+
+       if (!sub_ie_val) {
+               /* This subscriber doesn't care about this IE, so consider
+                * it matched. */
+               return 1;
+       }
+
+       switch (ie_val->ie_pltype) {
+       case AST_EVENT_IE_PLTYPE_UINT:
+               res = (ie_val->payload.uint != sub_ie_val->payload.uint);
+               break;
+       case AST_EVENT_IE_PLTYPE_STR:
+               res = strcmp(ie_val->payload.str, sub_ie_val->payload.str);
+               break;
+       case AST_EVENT_IE_PLTYPE_RAW:
+               res = memcmp(ie_val->payload.raw,
+                               sub_ie_val->payload.raw, ie_val->raw_datalen);
+               break;
+       case AST_EVENT_IE_PLTYPE_EXISTS:
+       case AST_EVENT_IE_PLTYPE_UNKNOWN:
+               break;
+       }
+
+       return res;
+}
+
 enum ast_event_subscriber_res ast_event_check_subscriber(enum ast_event_type type, ...)
 {
        va_list ap;
        enum ast_event_ie_type ie_type;
        enum ast_event_subscriber_res res = AST_EVENT_SUB_NONE;
-       struct ast_event_ie_val *ie_val, *sub_ie_val;
+       struct ast_event_ie_val *ie_val;
        struct ast_event_sub *sub;
        AST_LIST_HEAD_NOLOCK_STATIC(ie_vals, ast_event_ie_val);
+       const enum ast_event_type event_types[] = { type, AST_EVENT_ALL };
+       int i;
 
        if (type >= AST_EVENT_TOTAL) {
                ast_log(LOG_ERROR, "%u is an invalid type!\n", type);
@@ -357,47 +405,42 @@ enum ast_event_subscriber_res ast_event_check_subscriber(enum ast_event_type typ
        }
        va_end(ap);
 
-       AST_RWDLLIST_RDLOCK(&ast_event_subs[type]);
-       AST_RWDLLIST_TRAVERSE(&ast_event_subs[type], sub, entry) {
-               AST_LIST_TRAVERSE(&ie_vals, ie_val, entry) {
-                       AST_LIST_TRAVERSE(&sub->ie_vals, sub_ie_val, entry) {
-                               if (sub_ie_val->ie_type == ie_val->ie_type)
+       for (i = 0; i < ARRAY_LEN(event_types); i++) {
+               AST_RWDLLIST_RDLOCK(&ast_event_subs[event_types[i]]);
+               AST_RWDLLIST_TRAVERSE(&ast_event_subs[event_types[i]], sub, entry) {
+                       AST_LIST_TRAVERSE(&ie_vals, ie_val, entry) {
+                               if (match_ie_val_to_sub(sub, ie_val)) {
                                        break;
+                               }
                        }
-                       if (!sub_ie_val) {
-                               if (ie_val->ie_pltype == AST_EVENT_IE_PLTYPE_EXISTS)
-                                       break;
-                               continue;
-                       }
-                       /* The subscriber doesn't actually care what the value is */
-                       if (sub_ie_val->ie_pltype == AST_EVENT_IE_PLTYPE_EXISTS)
-                               continue;
-                       if (ie_val->ie_pltype == AST_EVENT_IE_PLTYPE_UINT &&
-                               ie_val->payload.uint != sub_ie_val->payload.uint)
-                               break;
-                       if (ie_val->ie_pltype == AST_EVENT_IE_PLTYPE_STR &&
-                               strcmp(ie_val->payload.str, sub_ie_val->payload.str))
-                               break;
-                       if (ie_val->ie_pltype == AST_EVENT_IE_PLTYPE_RAW &&
-                               memcmp(ie_val->payload.raw, sub_ie_val->payload.raw, ie_val->raw_datalen))
+
+                       if (!ie_val) {
+                               /* Everything matched. */
                                break;
+                       }
                }
-               if (!ie_val)
+               AST_RWDLLIST_UNLOCK(&ast_event_subs[event_types[i]]);
+               if (sub) {
                        break;
+               }
        }
-       AST_RWDLLIST_UNLOCK(&ast_event_subs[type]);
-
-       if (sub) /* All parameters were matched */
-               return AST_EVENT_SUB_EXISTS;
 
-       AST_RWDLLIST_RDLOCK(&ast_event_subs[AST_EVENT_ALL]);
-       if (!AST_DLLIST_EMPTY(&ast_event_subs[AST_EVENT_ALL]))
-               res = AST_EVENT_SUB_EXISTS;
-       AST_RWDLLIST_UNLOCK(&ast_event_subs[AST_EVENT_ALL]);
-
-       return res;
+       return sub ? AST_EVENT_SUB_EXISTS : AST_EVENT_SUB_NONE;
 }
 
+/*!
+ * \internal
+ * \brief Check if an ie_val matches an event
+ *
+ * \param event event to check against
+ * \param ie_val IE value to check
+ * \param event2 optional event, if specified, the value to compare against will be pulled
+ *        from this event instead of from the ie_val structure.  In this case, only the IE
+ *        type and payload type will be pulled from ie_val.
+ *
+ * \retval 0 not matched
+ * \retval non-zero matched
+ */
 static int match_ie_val(const struct ast_event *event,
                const struct ast_event_ie_val *ie_val, const struct ast_event *event2)
 {
@@ -1118,32 +1161,28 @@ static int handle_event(void *data)
 {
        struct ast_event_ref *event_ref = data;
        struct ast_event_sub *sub;
-       uint16_t host_event_type;
-
-       host_event_type = ntohs(event_ref->event->type);
+       const enum ast_event_type event_types[] = {
+               ntohs(event_ref->event->type),
+               AST_EVENT_ALL
+       };
+       int i;
 
-       /* Subscribers to this specific event first */
-       AST_RWDLLIST_RDLOCK(&ast_event_subs[host_event_type]);
-       AST_RWDLLIST_TRAVERSE(&ast_event_subs[host_event_type], sub, entry) {
-               struct ast_event_ie_val *ie_val;
-               AST_LIST_TRAVERSE(&sub->ie_vals, ie_val, entry) {
-                       if (!match_ie_val(event_ref->event, ie_val, NULL)) {
-                               break;
+       for (i = 0; i < ARRAY_LEN(event_types); i++) {
+               AST_RWDLLIST_RDLOCK(&ast_event_subs[event_types[i]]);
+               AST_RWDLLIST_TRAVERSE(&ast_event_subs[event_types[i]], sub, entry) {
+                       struct ast_event_ie_val *ie_val;
+                       AST_LIST_TRAVERSE(&sub->ie_vals, ie_val, entry) {
+                               if (!match_ie_val(event_ref->event, ie_val, NULL)) {
+                                       break;
+                               }
                        }
+                       if (ie_val) {
+                               continue;
+                       }
+                       sub->cb(event_ref->event, sub->userdata);
                }
-               if (ie_val) {
-                       continue;
-               }
-               sub->cb(event_ref->event, sub->userdata);
-       }
-       AST_RWDLLIST_UNLOCK(&ast_event_subs[host_event_type]);
-
-       /* Now to subscribers to all event types */
-       AST_RWDLLIST_RDLOCK(&ast_event_subs[AST_EVENT_ALL]);
-       AST_RWDLLIST_TRAVERSE(&ast_event_subs[AST_EVENT_ALL], sub, entry) {
-               sub->cb(event_ref->event, sub->userdata);
+               AST_RWDLLIST_UNLOCK(&ast_event_subs[event_types[i]]);
        }
-       AST_RWDLLIST_UNLOCK(&ast_event_subs[AST_EVENT_ALL]);
 
        ao2_ref(event_ref, -1);