Hi Miao,
On Tue, Aug 18, 2020 at 3:31 PM Miao-chen Chou <mcchou@xxxxxxxxxxxx> wrote:
>
> This implements the RegisterMonitor() method handler of ADV monitor
> manager interface.
>
> The following tests were performed.
> - Issue a RegisterMonitor() call with a valid path and expect a
> success as return.
> - Issue a RegisterMonitor() call with an invalid path and expect
> org.bluez.Error.InvalidArguments as return.
> - Issue two Registermonitor() calls with the same path and expect
> org.bluez.Error.AlreadyExists.
> - Verify the values of the registered paths with logging.
> - Verify D-Bus disconnection callback was triggered when the client detach
> from D-Bus.
>
> Reviewed-by: Yun-Hao Chung <howardchung@xxxxxxxxxx>
> Reviewed-by: Manish Mandlik <mmandlik@xxxxxxxxxxxx>
> Reviewed-by: Abhishek Pandit-Subedi <abhishekpandit@xxxxxxxxxxxx>
> ---
>
> src/adv_monitor.c | 167 +++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 166 insertions(+), 1 deletion(-)
>
> diff --git a/src/adv_monitor.c b/src/adv_monitor.c
> index 4d02237e8..3d27ad18b 100644
> --- a/src/adv_monitor.c
> +++ b/src/adv_monitor.c
> @@ -22,7 +22,9 @@
> #endif
>
> #define _GNU_SOURCE
> +#include <errno.h>
> #include <stdint.h>
> +#include <string.h>
>
> #include <glib.h>
> #include <dbus/dbus.h>
> @@ -34,7 +36,9 @@
> #include "adapter.h"
> #include "dbus-common.h"
> #include "log.h"
> +#include "src/error.h"
> #include "src/shared/mgmt.h"
> +#include "src/shared/queue.h"
> #include "src/shared/util.h"
>
> #include "adv_monitor.h"
> @@ -52,12 +56,170 @@ struct btd_adv_monitor_manager {
> uint16_t max_num_monitors;
> uint8_t max_num_patterns;
>
> + struct queue *apps; /* apps who registered for Adv monitoring */
> };
>
> +struct adv_monitor_app {
> + struct btd_adv_monitor_manager *manager;
> + char *owner;
> + char *path;
> +
> + DBusMessage *reg;
> + GDBusClient *client;
> +};
> +
> +struct app_match_data {
> + const char *owner;
> + const char *path;
> +};
> +
> +/* Replies to an app's D-Bus message and unref it */
> +static void app_reply_msg(struct adv_monitor_app *app, DBusMessage *reply)
> +{
> + if (!app || !app->reg || !reply)
> + return;
> +
> + g_dbus_send_message(btd_get_dbus_connection(), reply);
> + dbus_message_unref(app->reg);
> + app->reg = NULL;
> +}
> +
> +/* Destroys an app object along with related D-Bus handlers */
> +static void app_destroy(void *data)
> +{
> + struct adv_monitor_app *app = data;
> +
> + if (!app)
> + return;
> +
> + DBG("Destroy Adv Monitor app %s at path %s", app->owner, app->path);
> +
> + if (app->reg) {
> + app_reply_msg(app, btd_error_failed(app->reg,
> + "Adv Monitor app destroyed"));
> + }
> +
> + if (app->client) {
> + g_dbus_client_set_disconnect_watch(app->client, NULL, NULL);
> + g_dbus_client_set_proxy_handlers(app->client, NULL, NULL, NULL,
> + NULL);
> + g_dbus_client_set_ready_watch(app->client, NULL, NULL);
> + g_dbus_client_unref(app->client);
> + app->client = NULL;
> + }
> +
> + g_free(app->owner);
> + app->owner = NULL;
> + g_free(app->path);
> + app->path = NULL;
Same comment as before, if the whole object would be free then there
is no need to reset each individual field.
> +
> + g_free(app);
> +}
> +
> +/* Handles a D-Bus disconnection event of an app */
> +static void app_disconnect_cb(DBusConnection *conn, void *user_data)
> +{
> + struct adv_monitor_app *app = user_data;
> +
> + btd_info(app->manager->adapter_id, "Adv Monitor app %s disconnected "
> + "from D-Bus", app->owner);
> + if (app && queue_remove(app->manager->apps, app))
> + app_destroy(app);
> +}
> +
> +/* Creates an app object, initiates it and sets D-Bus event handlers */
> +static struct adv_monitor_app *app_create(DBusConnection *conn,
> + const char *sender, const char *path,
> + struct btd_adv_monitor_manager *manager)
> +{
> + struct adv_monitor_app *app;
> +
> + if (!path || !sender || !manager)
> + return NULL;
> +
> + app = g_new0(struct adv_monitor_app, 1);
Please use new0 instead of g_new0 in new code.
> + if (!app)
> + return NULL;
> +
> + app->owner = g_strdup(sender);
> + app->path = g_strdup(path);
> + app->manager = manager;
> + app->reg = NULL;
> +
> + app->client = g_dbus_client_new(conn, sender, path);
> + if (!app->client) {
> + app_destroy(app);
> + return NULL;
> + }
> +
> + g_dbus_client_set_disconnect_watch(app->client, app_disconnect_cb, app);
> + g_dbus_client_set_proxy_handlers(app->client, NULL, NULL, NULL, NULL);
> + g_dbus_client_set_ready_watch(app->client, NULL, NULL);
> +
> + return app;
> +}
> +
> +/* Matches an app based on its owner and path */
> +static bool app_match(const void *a, const void *b)
> +{
> + const struct adv_monitor_app *app = a;
> + const struct app_match_data *match = b;
> +
> + if (match->owner && strcmp(app->owner, match->owner))
> + return false;
> +
> + if (match->path && strcmp(app->path, match->path))
> + return false;
> +
> + return true;
> +}
> +
> +/* Handles a RegisterMonitor D-Bus call */
> +static DBusMessage *register_monitor(DBusConnection *conn, DBusMessage *msg,
> + void *user_data)
> +{
> + DBusMessageIter args;
> + struct app_match_data match;
> + struct adv_monitor_app *app;
> + struct btd_adv_monitor_manager *manager = user_data;
> +
> + if (!dbus_message_iter_init(msg, &args))
> + return btd_error_invalid_args(msg);
> +
> + if (dbus_message_iter_get_arg_type(&args) != DBUS_TYPE_OBJECT_PATH)
> + return btd_error_invalid_args(msg);
> +
> + dbus_message_iter_get_basic(&args, &match.path);
> +
> + if (!strlen(match.path) || !g_str_has_prefix(match.path, "/"))
> + return btd_error_invalid_args(msg);
> +
> + match.owner = dbus_message_get_sender(msg);
> +
> + if (queue_find(manager->apps, app_match, &match))
> + return btd_error_already_exists(msg);
> +
> + app = app_create(conn, match.owner, match.path, manager);
> + if (!app) {
> + btd_error(manager->adapter_id,
> + "Failed to reserve %s for Adv Monitor app %s",
> + match.path, match.owner);
> + return btd_error_failed(msg,
> + "Failed to create Adv Monitor app");
> + }
> +
> + queue_push_tail(manager->apps, app);
> +
> + btd_info(manager->adapter_id, "Path %s reserved for Adv Monitor app %s",
> + match.path, match.owner);
> +
> + return dbus_message_new_method_return(msg);
> +}
> +
> static const GDBusMethodTable adv_monitor_methods[] = {
> { GDBUS_METHOD("RegisterMonitor",
> GDBUS_ARGS({ "application", "o" }),
> - NULL, NULL) },
> + NULL, register_monitor) },
> { GDBUS_ASYNC_METHOD("UnregisterMonitor",
> GDBUS_ARGS({ "application", "o" }),
> NULL, NULL) },
> @@ -157,6 +319,7 @@ static struct btd_adv_monitor_manager *manager_new(
> manager->mgmt = mgmt_ref(mgmt);
> manager->adapter_id = btd_adapter_get_index(adapter);
> manager->path = g_strdup(adapter_get_path(manager->adapter));
> + manager->apps = queue_new();
>
> return manager;
> }
> @@ -170,6 +333,8 @@ static void manager_free(struct btd_adv_monitor_manager *manager)
> g_free(manager->path);
> manager->path = NULL;
>
> + queue_destroy(manager->apps, app_destroy);
> +
> g_free(manager);
> }
>
> --
> 2.26.2
>
--
Luiz Augusto von Dentz
