weston-log/weston-log-wayland: Inline private subscription functions

This avoids duplicated bits, by calling the scopes's callback (if any)
and adding the subscription to the scope's subscription list. Further
more, the scope's name when creating the subscription is not needed so
removed that as well.

In mirror, also inline removing of subscription for scope's subscription
list. Fix a potential corner case when the user can request a
subscription to an invalid scope in stream_destroy().

Signed-off-by: Marius Vlad <marius.vlad@collabora.com>

3 files changed, 35 insertions, 32 deletions

diff --git a/libweston/weston-log-internal.h b/libweston/weston-log-internal.h
index d93195c6..d8023358 100644
--- a/libweston/weston-log-internal.h
+++ b/libweston/weston-log-internal.h
@@ -27,6 +27,8 @@
 
 #include "wayland-util.h"
 
+struct weston_log_subscription;
+
 /** Subscriber allows each type of stream to customize or to provide its own
  * methods to manipulate the underlying storage. It follows also an
  * object-oriented approach, contains the ops callbacks and a list of
@@ -61,9 +63,9 @@ struct weston_log_subscriber {
 	struct wl_list subscription_list;       /**< weston_log_subscription::owner_link */
 };
 
-struct weston_log_subscription *
+void
 weston_log_subscription_create(struct weston_log_subscriber *owner,
-			       const char *scope_name);
+			       struct weston_log_scope *scope);
 
 void
 weston_log_subscription_destroy(struct weston_log_subscription *sub);
diff --git a/libweston/weston-log-wayland.c b/libweston/weston-log-wayland.c
index c5e30fca..f620e64d 100644
--- a/libweston/weston-log-wayland.c
+++ b/libweston/weston-log-wayland.c
@@ -171,7 +171,6 @@ stream_create(struct weston_log_context *log_ctx, const char *name,
 {
 	struct weston_log_debug_wayland *stream;
 	struct weston_log_scope *scope;
-	struct weston_log_subscription *sub;
 
 	stream = zalloc(sizeof *stream);
 	if (!stream)
@@ -185,12 +184,9 @@ stream_create(struct weston_log_context *log_ctx, const char *name,
 	stream->base.complete = weston_log_debug_wayland_complete;
 	wl_list_init(&stream->base.subscription_list);
 
-
 	scope = weston_log_get_scope(log_ctx, name);
 	if (scope) {
-		sub = weston_log_subscription_create(&stream->base, name);
-		weston_log_subscription_add(scope, sub);
-		weston_log_run_begin_cb(scope);
+		weston_log_subscription_create(&stream->base, scope);
 	} else {
 		stream_close_on_failure(stream,
 					"Debug stream name '%s' is unknown.",
@@ -212,8 +208,11 @@ stream_destroy(struct wl_resource *stream_resource)
 		close(stream->fd);
 
 	sub = weston_log_subscriber_get_only_subscription(&stream->base);
-	weston_log_subscription_remove(sub);
-	weston_log_subscription_destroy(sub);
+
+	/* we can have a zero subscription if clients tried to subscribe
+	 * to a non-existent scope */
+	if (sub)
+		weston_log_subscription_destroy(sub);
 
 	free(stream);
 }
diff --git a/libweston/weston-log.c b/libweston/weston-log.c
index 566a4fac..9caca22d 100644
--- a/libweston/weston-log.c
+++ b/libweston/weston-log.c
@@ -170,9 +170,13 @@ weston_log_subscription_destroy_pending(struct weston_log_subscription *sub)
  * Destroying the subscription using weston_log_subscription_destroy() will
  * remove the link from the subscription list and free storage alloc'ed.
  *
+ * Note that this adds the subscription to the scope's subscription list
+ * hence the \c scope required argument.
+ *
  * @param owner the subscriber owner, must be created before creating a
  * subscription
- * @param scope_name the scope for which to create this subscription
+ * @param scope the scope in order to add the subscription to the scope's
+ * subscription list
  * @returns a weston_log_subscription object in case of success, or NULL
  * otherwise
  *
@@ -180,35 +184,43 @@ weston_log_subscription_destroy_pending(struct weston_log_subscription *sub)
  * @sa weston_log_subscription_destroy, weston_log_subscription_remove,
  * weston_log_subscription_add
  */
-struct weston_log_subscription *
+void
 weston_log_subscription_create(struct weston_log_subscriber *owner,
-			       const char *scope_name)
+			       struct weston_log_scope *scope)
 {
 	struct weston_log_subscription *sub;
-	assert(scope_name);
 	assert(owner);
+	assert(scope);
+	assert(scope->name);
 
 	sub = zalloc(sizeof(*sub));
 	if (!sub)
-		return NULL;
+		return;
 
 	sub->owner = owner;
-	sub->scope_name = strdup(scope_name);
+	sub->scope_name = strdup(scope->name);
 
 	wl_list_insert(&sub->owner->subscription_list, &sub->owner_link);
-	return sub;
+
+	weston_log_subscription_add(scope, sub);
+	weston_log_run_begin_cb(scope);
 }
 
 /** Destroys the subscription
  *
- * @param sub
- * @internal
+ * Removes the subscription from the scopes subscription list and from
+ * subscriber's subscription list. It destroys the subscription afterwads.
+ *
+ * @memberof weston_log_subscription
  */
 void
 weston_log_subscription_destroy(struct weston_log_subscription *sub)
 {
+	assert(sub);
 	if (sub->owner)
 		wl_list_remove(&sub->owner_link);
+
+	weston_log_subscription_remove(sub);
 	free(sub->scope_name);
 	free(sub);
 }
@@ -544,12 +556,7 @@ weston_compositor_add_log_scope(struct weston_log_context *log_ctx,
 
 	/* check if there are any pending subscriptions to this scope */
 	while ((pending_sub = find_pending_subscription(log_ctx, scope->name)) != NULL) {
-		struct weston_log_subscription *sub =
-			weston_log_subscription_create(pending_sub->owner,
-						       scope->name);
-
-		weston_log_subscription_add(scope, sub);
-		weston_log_run_begin_cb(scope);
+		weston_log_subscription_create(pending_sub->owner, scope);
 
 		/* remove it from pending */
 		weston_log_subscription_destroy_pending(pending_sub);
@@ -581,7 +588,6 @@ weston_compositor_log_scope_destroy(struct weston_log_scope *scope)
 		if (sub->owner->destroy)
 			sub->owner->destroy(sub->owner);
 
-		weston_log_subscription_remove(sub);
 		weston_log_subscription_destroy(sub);
 	}
 
@@ -793,18 +799,14 @@ weston_log_subscribe(struct weston_log_context *log_ctx,
 	assert(scope_name);
 
 	struct weston_log_scope *scope;
-	struct weston_log_subscription *sub;
 
 	scope = weston_log_get_scope(log_ctx, scope_name);
-	if (scope) {
-		sub = weston_log_subscription_create(subscriber, scope_name);
-		weston_log_subscription_add(scope, sub);
-		weston_log_run_begin_cb(scope);
-	} else {
+	if (scope)
+		weston_log_subscription_create(subscriber, scope);
+	else
 		/*
 		 * if we don't have already as scope for it, add it to pending
 		 * subscription list
 		 */
 		weston_log_subscription_create_pending(subscriber, scope_name, log_ctx);
-	}
 }