event-rule: kernel probe: force location on create

Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I485ab3bb32317d97ad46f7de71c72a37afcd64c2

diff --git a/include/lttng/event-rule/kernel-probe.h b/include/lttng/event-rule/kernel-probe.h
index 2d5257512094df451890ea25bd28ac8e2d395202..e9272ff63afc1d5333e8d56a734819e4c8ece4a5 100644 (file)
--- a/include/lttng/event-rule/kernel-probe.h
+++ b/include/lttng/event-rule/kernel-probe.h
@@ -19,21 +19,12 @@ struct lttng_kernel_probe_location;
 /*
  * Create a newly allocated kernel probe event rule.
  *
- * Returns a new event rule on success, NULL on failure. The returned event rule
- * must be destroyed using lttng_event_rule_destroy().
- */
-extern struct lttng_event_rule *lttng_event_rule_kernel_probe_create(void);
-
-/*
- * Set the kernel probe location of a kernel probe event rule.
- *
  * The location is copied internally.
  *
- * Returns LTTNG_EVENT_RULE_STATUS_OK on success, LTTNG_EVENT_RULE_STATUS_INVALID
- * if invalid parameters are passed.
+ * Returns a new event rule on success, NULL on failure. The returned event rule
+ * must be destroyed using lttng_event_rule_destroy().
  */
-extern enum lttng_event_rule_status lttng_event_rule_kernel_probe_set_location(
-               struct lttng_event_rule *rule,
+extern struct lttng_event_rule *lttng_event_rule_kernel_probe_create(
                const struct lttng_kernel_probe_location *location);
 
 /*

diff --git a/src/bin/lttng/commands/add_trigger.c b/src/bin/lttng/commands/add_trigger.c
index 19d3a6f3a584351c06cf6bed134365af99a35f08..ec629cb30750c5b81a46b7174540d10247a38264 100644 (file)
--- a/src/bin/lttng/commands/add_trigger.c
+++ b/src/bin/lttng/commands/add_trigger.c
@@ -960,11 +960,6 @@ struct parse_event_rule_res parse_event_rule(int *argc, const char ***argv)
                int ret;
                enum lttng_event_rule_status event_rule_status;
 
-               res.er = lttng_event_rule_kernel_probe_create();
-               if (!res.er) {
-                       ERR("Failed to create kprobe event rule.");
-                       goto error;
-               }
 
                ret = parse_kernel_probe_opts(source, &kernel_probe_location);
                if (ret) {
@@ -972,16 +967,16 @@ struct parse_event_rule_res parse_event_rule(int *argc, const char ***argv)
                        goto error;
                }
 
-               event_rule_status = lttng_event_rule_kernel_probe_set_event_name(res.er, tracepoint_name);
-               if (event_rule_status != LTTNG_EVENT_RULE_STATUS_OK) {
-                       ERR("Failed to set kprobe event rule's name to '%s'.", tracepoint_name);
+               assert(kernel_probe_location);
+               res.er = lttng_event_rule_kernel_probe_create(kernel_probe_location);
+               if (!res.er) {
+                       ERR("Failed to create kprobe event rule.");
                        goto error;
                }
 
-               assert(kernel_probe_location);
-               event_rule_status = lttng_event_rule_kernel_probe_set_location(res.er, kernel_probe_location);
+               event_rule_status = lttng_event_rule_kernel_probe_set_event_name(res.er, tracepoint_name);
                if (event_rule_status != LTTNG_EVENT_RULE_STATUS_OK) {
-                       ERR("Failed to set kprobe event rule's location.");
+                       ERR("Failed to set kprobe event rule's name to '%s'.", tracepoint_name);
                        goto error;
                }
 

diff --git a/src/common/event-rule/kernel-probe.c b/src/common/event-rule/kernel-probe.c
index 7506c2f11850ad014f9122ceddfc7f08dc846d60..f53e9245bd5de617230d8c67e632a2dd7c70a491 100644 (file)
--- a/src/common/event-rule/kernel-probe.c
+++ b/src/common/event-rule/kernel-probe.c
@@ -16,6 +16,7 @@
 #include <common/hashtable/utils.h>
 #include <ctype.h>
 #include <lttng/constant.h>
+#include <lttng/event-rule/event-rule.h>
 #include <lttng/event-rule/event-rule-internal.h>
 #include <lttng/event-rule/kernel-probe-internal.h>
 #include <lttng/kernel-probe.h>
@@ -195,7 +196,35 @@ lttng_event_rule_kernel_probe_hash(
        return hash;
 }
 
-struct lttng_event_rule *lttng_event_rule_kernel_probe_create(void)
+static
+int kernel_probe_set_location(
+               struct lttng_event_rule_kernel_probe *kprobe,
+               const struct lttng_kernel_probe_location *location)
+{
+       int ret;
+       struct lttng_kernel_probe_location *location_copy = NULL;
+
+       if (!kprobe || !location || kprobe->location) {
+               ret = -1;
+               goto end;
+       }
+
+       location_copy = lttng_kernel_probe_location_copy(location);
+       if (!location_copy) {
+               ret = -1;
+               goto end;
+       }
+
+       kprobe->location = location_copy;
+       location_copy = NULL;
+       ret = 0;
+end:
+       lttng_kernel_probe_location_destroy(location_copy);
+       return ret;
+}
+
+struct lttng_event_rule *lttng_event_rule_kernel_probe_create(
+               const struct lttng_kernel_probe_location *location)
 {
        struct lttng_event_rule *rule = NULL;
        struct lttng_event_rule_kernel_probe *krule;
@@ -219,6 +248,12 @@ struct lttng_event_rule *lttng_event_rule_kernel_probe_create(void)
        krule->parent.generate_exclusions =
                        lttng_event_rule_kernel_probe_generate_exclusions;
        krule->parent.hash = lttng_event_rule_kernel_probe_hash;
+
+       if (kernel_probe_set_location(krule, location)) {
+               lttng_event_rule_destroy(rule);
+               rule = NULL;
+       }
+
 end:
        return rule;
 }
@@ -234,8 +269,7 @@ ssize_t lttng_event_rule_kernel_probe_create_from_payload(
        const char *name;
        struct lttng_buffer_view current_buffer_view;
        struct lttng_event_rule *rule = NULL;
-       struct lttng_event_rule_kernel_probe *kprobe = NULL;
-       struct lttng_kernel_probe_location *location;
+       struct lttng_kernel_probe_location *location = NULL;
 
        if (!_event_rule) {
                ret = -1;
@@ -252,15 +286,6 @@ ssize_t lttng_event_rule_kernel_probe_create_from_payload(
 
        kprobe_comm = (typeof(kprobe_comm)) current_buffer_view.data;
 
-       rule = lttng_event_rule_kernel_probe_create();
-       if (!rule) {
-               ERR("Failed to create event rule kprobe.");
-               ret = -1;
-               goto end;
-       }
-
-       kprobe = container_of(rule, struct lttng_event_rule_kernel_probe, parent);
-
        /* Skip to payload */
        offset += current_buffer_view.size;
 
@@ -311,11 +336,16 @@ ssize_t lttng_event_rule_kernel_probe_create_from_payload(
                goto end;
        }
 
-       kprobe->location = location;
-
        /* Skip after the location */
        offset += kprobe_comm->location_len;
 
+       rule = lttng_event_rule_kernel_probe_create(location);
+       if (!rule) {
+               ERR("Failed to create event rule kprobe.");
+               ret = -1;
+               goto end;
+       }
+
        status = lttng_event_rule_kernel_probe_set_event_name(rule, name);
        if (status != LTTNG_EVENT_RULE_STATUS_OK) {
                ERR("Failed to set event rule kprobe name.");
@@ -327,41 +357,11 @@ ssize_t lttng_event_rule_kernel_probe_create_from_payload(
        rule = NULL;
        ret = offset;
 end:
+       lttng_kernel_probe_location_destroy(location);
        lttng_event_rule_destroy(rule);
        return ret;
 }
 
-enum lttng_event_rule_status lttng_event_rule_kernel_probe_set_location(
-               struct lttng_event_rule *rule,
-               const struct lttng_kernel_probe_location *location)
-{
-       struct lttng_kernel_probe_location *location_copy = NULL;
-       struct lttng_event_rule_kernel_probe *kprobe;
-       enum lttng_event_rule_status status = LTTNG_EVENT_RULE_STATUS_OK;
-
-       if (!rule || !IS_KPROBE_EVENT_RULE(rule) || !location) {
-               status = LTTNG_EVENT_RULE_STATUS_INVALID;
-               goto end;
-       }
-
-       kprobe = container_of(rule, struct lttng_event_rule_kernel_probe, parent);
-       location_copy = lttng_kernel_probe_location_copy(location);
-       if (!location_copy) {
-               status = LTTNG_EVENT_RULE_STATUS_ERROR;
-               goto end;
-       }
-
-       if (kprobe->location) {
-               lttng_kernel_probe_location_destroy(kprobe->location);
-       }
-
-       kprobe->location = location_copy;
-       location_copy = NULL;
-end:
-       lttng_kernel_probe_location_destroy(location_copy);
-       return status;
-}
-
 enum lttng_event_rule_status lttng_event_rule_kernel_probe_get_location(
                const struct lttng_event_rule *rule,
                const struct lttng_kernel_probe_location **location)

diff --git a/tests/regression/tools/notification/notification.c b/tests/regression/tools/notification/notification.c
index 0f1a418112eefedb41401b5c89174b3c9baf4feb..e1d57f9635fb15a5ac31bf80810b149a840593a4 100644 (file)
--- a/tests/regression/tools/notification/notification.c
+++ b/tests/regression/tools/notification/notification.c
@@ -1864,14 +1864,9 @@ static void test_kprobe_event_rule_notification(
                        lttng_session_daemon_notification_endpoint);
        ok(notification_channel, "Notification channel object creation");
 
-       event_rule = lttng_event_rule_kernel_probe_create();
+       event_rule = lttng_event_rule_kernel_probe_create(location);
        ok(event_rule, "kprobe event rule object creation");
 
-       event_rule_status = lttng_event_rule_kernel_probe_set_location(
-                       event_rule, location);
-       ok(event_rule_status == LTTNG_EVENT_RULE_STATUS_OK,
-                       "Setting kprobe event rule location: '%s'", symbol_name);
-
        event_rule_status = lttng_event_rule_kernel_probe_set_event_name(
                        event_rule, trigger_name);
        ok(event_rule_status == LTTNG_EVENT_RULE_STATUS_OK,
@@ -2600,7 +2595,7 @@ int main(int argc, const char *argv[])
        }
        case 4:
        {
-               plan_tests(13);
+               plan_tests(12);
                /* Test cases that need the kernel tracer. */
                assert(domain_type == LTTNG_DOMAIN_KERNEL);
 

diff --git a/tests/unit/test_event_rule.c b/tests/unit/test_event_rule.c
index 07614f4b88e67ba057fdff507e6d99029568cc3e..49aade24d8bf184a8b457f7d5515f0bc4760c91d 100644 (file)
--- a/tests/unit/test_event_rule.c
+++ b/tests/unit/test_event_rule.c
@@ -37,7 +37,7 @@ int lttng_opt_quiet = 1;
 int lttng_opt_verbose;
 int lttng_opt_mi;
 
-#define NUM_TESTS 187
+#define NUM_TESTS 185
 
 struct tracepoint_test {
        enum lttng_domain_type type;
@@ -323,12 +323,9 @@ static void test_event_rule_kernel_probe_by_location(
 
        lttng_payload_init(&payload);
 
-       kprobe = lttng_event_rule_kernel_probe_create();
+       kprobe = lttng_event_rule_kernel_probe_create(location);
        ok(kprobe, "kprobe event rule object creation.");
 
-       status = lttng_event_rule_kernel_probe_set_location(kprobe, location);
-       ok(status == LTTNG_EVENT_RULE_STATUS_OK,
-                       "Setting kprobe event rule location.");
        status = lttng_event_rule_kernel_probe_get_location(kprobe, &_location);
        ok(status == LTTNG_EVENT_RULE_STATUS_OK,
                        "Getting kprobe event rule location.");