Fix: provide more precise error report for enable event

Fixes #658

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>

diff --git a/lttng-abi.c b/lttng-abi.c
index a373504c26d0d7804e22cafb4e0dcac3747212a9..1cc9510110dbc216ecf02c4ca8157dfc8f12a78e 100644 (file)
--- a/lttng-abi.c
+++ b/lttng-abi.c
@@ -43,6 +43,7 @@
 #include <linux/file.h>
 #include <linux/uaccess.h>
 #include <linux/slab.h>
+#include <linux/err.h>
 #include "wrapper/vmalloc.h"   /* for wrapper_vmalloc_sync_all() */
 #include "wrapper/ringbuffer/vfs.h"
 #include "wrapper/ringbuffer/backend.h"
@@ -912,8 +913,9 @@ int lttng_abi_create_event(struct file *channel_file,
                 * will stay invariant for the rest of the session.
                 */
                event = lttng_event_create(channel, event_param, NULL, NULL);
-               if (!event) {
-                       ret = -EINVAL;
+               WARN_ON_ONCE(!event);
+               if (IS_ERR(event)) {
+                       ret = PTR_ERR(event);
                        goto event_error;
                }
                event_file->private_data = event;

diff --git a/lttng-events.c b/lttng-events.c
index e3a7324b84ab82ebb31761500d812c0c1c728f80..97d82a297b911d9ad945cc6b88e484d87d2d8fb3 100644 (file)
--- a/lttng-events.c
+++ b/lttng-events.c
@@ -27,6 +27,7 @@
 #include <linux/slab.h>
 #include <linux/jiffies.h>
 #include <linux/utsname.h>
+#include <linux/err.h>
 #include "wrapper/uuid.h"
 #include "wrapper/vmalloc.h"   /* for wrapper_vmalloc_sync_all() */
 #include "wrapper/random.h"
@@ -366,18 +367,25 @@ struct lttng_event *lttng_event_create(struct lttng_channel *chan,
        int ret;
 
        mutex_lock(&sessions_mutex);
-       if (chan->free_event_id == -1U)
+       if (chan->free_event_id == -1U) {
+               event = ERR_PTR(-EMFILE);
                goto full;
+       }
        /*
         * This is O(n^2) (for each event, the loop is called at event
         * creation). Might require a hash if we have lots of events.
         */
-       list_for_each_entry(event, &chan->session->events, list)
-               if (!strcmp(event->desc->name, event_param->name))
+       list_for_each_entry(event, &chan->session->events, list) {
+               if (!strcmp(event->desc->name, event_param->name)) {
+                       event = ERR_PTR(-EEXIST);
                        goto exist;
+               }
+       }
        event = kmem_cache_zalloc(event_cache, GFP_KERNEL);
-       if (!event)
+       if (!event) {
+               event = ERR_PTR(-ENOMEM);
                goto cache_error;
+       }
        event->chan = chan;
        event->filter = filter;
        event->id = chan->free_event_id++;
@@ -388,13 +396,17 @@ struct lttng_event *lttng_event_create(struct lttng_channel *chan,
        switch (event_param->instrumentation) {
        case LTTNG_KERNEL_TRACEPOINT:
                event->desc = lttng_event_get(event_param->name);
-               if (!event->desc)
+               if (!event->desc) {
+                       event = ERR_PTR(-ENOENT);
                        goto register_error;
+               }
                ret = kabi_2635_tracepoint_probe_register(event_param->name,
                                event->desc->probe_callback,
                                event);
-               if (ret)
+               if (ret) {
+                       event = ERR_PTR(-EINVAL);
                        goto register_error;
+               }
                break;
        case LTTNG_KERNEL_KPROBE:
                ret = lttng_kprobes_register(event_param->name,
@@ -402,8 +414,10 @@ struct lttng_event *lttng_event_create(struct lttng_channel *chan,
                                event_param->u.kprobe.offset,
                                event_param->u.kprobe.addr,
                                event);
-               if (ret)
+               if (ret) {
+                       event = ERR_PTR(-EINVAL);
                        goto register_error;
+               }
                ret = try_module_get(event->desc->owner);
                WARN_ON_ONCE(!ret);
                break;
@@ -414,8 +428,10 @@ struct lttng_event *lttng_event_create(struct lttng_channel *chan,
                /* kretprobe defines 2 events */
                event_return =
                        kmem_cache_zalloc(event_cache, GFP_KERNEL);
-               if (!event_return)
+               if (!event_return) {
+                       event = ERR_PTR(-ENOMEM);
                        goto register_error;
+               }
                event_return->chan = chan;
                event_return->filter = filter;
                event_return->id = chan->free_event_id++;
@@ -432,6 +448,7 @@ struct lttng_event *lttng_event_create(struct lttng_channel *chan,
                                event, event_return);
                if (ret) {
                        kmem_cache_free(event_cache, event_return);
+                       event = ERR_PTR(-EINVAL);
                        goto register_error;
                }
                /* Take 2 refs on the module: one per event. */
@@ -441,10 +458,12 @@ struct lttng_event *lttng_event_create(struct lttng_channel *chan,
                WARN_ON_ONCE(!ret);
                ret = _lttng_event_metadata_statedump(chan->session, chan,
                                                    event_return);
+               WARN_ON_ONCE(ret > 0);
                if (ret) {
                        kmem_cache_free(event_cache, event_return);
                        module_put(event->desc->owner);
                        module_put(event->desc->owner);
+                       event = ERR_PTR(ret);
                        goto statedump_error;
                }
                list_add(&event_return->list, &chan->session->events);
@@ -454,23 +473,31 @@ struct lttng_event *lttng_event_create(struct lttng_channel *chan,
                ret = lttng_ftrace_register(event_param->name,
                                event_param->u.ftrace.symbol_name,
                                event);
-               if (ret)
+               if (ret) {
+                       event = ERR_PTR(ret);
                        goto register_error;
+               }
                ret = try_module_get(event->desc->owner);
                WARN_ON_ONCE(!ret);
                break;
        case LTTNG_KERNEL_NOOP:
                event->desc = internal_desc;
-               if (!event->desc)
+               if (!event->desc) {
+                       event = ERR_PTR(-EINVAL);
                        goto register_error;
+               }
                break;
        default:
                WARN_ON_ONCE(1);
+               event = ERR_PTR(-EINVAL);
                goto register_error;
        }
        ret = _lttng_event_metadata_statedump(chan->session, chan, event);
-       if (ret)
+       WARN_ON_ONCE(ret > 0);
+       if (ret) {
+               event = ERR_PTR(ret);
                goto statedump_error;
+       }
        list_add(&event->list, &chan->session->events);
        mutex_unlock(&sessions_mutex);
        return event;
@@ -483,7 +510,7 @@ cache_error:
 exist:
 full:
        mutex_unlock(&sessions_mutex);
-       return NULL;
+       return event;
 }
 
 /*

diff --git a/lttng-syscalls.c b/lttng-syscalls.c
index 62ed24aa66723c2cb16b8abe2a83cd6b5a8ccb48..3215564161c1c35987a815106b8638e66db0e745 100644 (file)
--- a/lttng-syscalls.c
+++ b/lttng-syscalls.c
@@ -23,6 +23,7 @@
 #include <linux/module.h>
 #include <linux/slab.h>
 #include <linux/compat.h>
+#include <linux/err.h>
 #include <asm/ptrace.h>
 #include <asm/syscall.h>
 
@@ -322,14 +323,15 @@ int fill_table(const struct trace_syscall_entry *table, size_t table_len,
                ev.instrumentation = LTTNG_KERNEL_NOOP;
                chan_table[i] = lttng_event_create(chan, &ev, filter,
                                                desc);
-               if (!chan_table[i]) {
+               WARN_ON_ONCE(!chan_table[i]);
+               if (IS_ERR(chan_table[i])) {
                        /*
                         * If something goes wrong in event registration
                         * after the first one, we have no choice but to
                         * leave the previous events in there, until
                         * deleted by session teardown.
                         */
-                       return -EINVAL;
+                       return PTR_ERR(chan_table[i]);
                }
        }
        return 0;
@@ -369,8 +371,9 @@ int lttng_syscalls_register(struct lttng_channel *chan, void *filter)
                ev.instrumentation = LTTNG_KERNEL_NOOP;
                chan->sc_unknown = lttng_event_create(chan, &ev, filter,
                                                    desc);
-               if (!chan->sc_unknown) {
-                       return -EINVAL;
+               WARN_ON_ONCE(!chan->sc_unknown);
+               if (IS_ERR(chan->sc_unknown)) {
+                       return PTR_ERR(chan->sc_unknown);
                }
        }
 
@@ -384,8 +387,9 @@ int lttng_syscalls_register(struct lttng_channel *chan, void *filter)
                ev.instrumentation = LTTNG_KERNEL_NOOP;
                chan->sc_compat_unknown = lttng_event_create(chan, &ev, filter,
                                                           desc);
-               if (!chan->sc_compat_unknown) {
-                       return -EINVAL;
+               WARN_ON_ONCE(!chan->sc_unknown);
+               if (IS_ERR(chan->sc_compat_unknown)) {
+                       return PTR_ERR(chan->sc_compat_unknown);
                }
        }
 
@@ -399,8 +403,9 @@ int lttng_syscalls_register(struct lttng_channel *chan, void *filter)
                ev.instrumentation = LTTNG_KERNEL_NOOP;
                chan->sc_exit = lttng_event_create(chan, &ev, filter,
                                                 desc);
-               if (!chan->sc_exit) {
-                       return -EINVAL;
+               WARN_ON_ONCE(!chan->sc_exit);
+               if (IS_ERR(chan->sc_exit)) {
+                       return PTR_ERR(chan->sc_exit);
                }
        }