From 602a6d403fc83a01f2f21fffbd3951197eda91de Mon Sep 17 00:00:00 2001 From: Jonathan Rajotte Date: Thu, 5 Nov 2020 12:02:32 -0500 Subject: [PATCH] event-rule: kernel probe: force location on create MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Signed-off-by: Jonathan Rajotte Signed-off-by: Jérémie Galarneau Change-Id: I485ab3bb32317d97ad46f7de71c72a37afcd64c2 --- include/lttng/event-rule/kernel-probe.h | 15 +--- src/bin/lttng/commands/add_trigger.c | 17 ++-- src/common/event-rule/kernel-probe.c | 90 +++++++++---------- .../tools/notification/notification.c | 9 +- tests/unit/test_event_rule.c | 7 +- 5 files changed, 58 insertions(+), 80 deletions(-) diff --git a/include/lttng/event-rule/kernel-probe.h b/include/lttng/event-rule/kernel-probe.h index 2d5257512..e9272ff63 100644 --- 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 19d3a6f3a..ec629cb30 100644 --- 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 7506c2f11..f53e9245b 100644 --- a/src/common/event-rule/kernel-probe.c +++ b/src/common/event-rule/kernel-probe.c @@ -16,6 +16,7 @@ #include #include #include +#include #include #include #include @@ -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 0f1a41811..e1d57f963 100644 --- 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 07614f4b8..49aade24d 100644 --- 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."); -- 2.34.1