From ff412fb523eeacd9f2b698ccefc4f716ace9afd0 Mon Sep 17 00:00:00 2001 From: Mathieu Desnoyers Date: Sun, 11 Dec 2011 10:20:01 -0500 Subject: [PATCH] Print compiler warning/runtime warning and truncate too long tracepoint names - also apply to loglevel names. - -Wsystem-headers needs to be used to get gcc to show the warnings from system headers (which is the category in which the tracepoint event declaration headers falls into). Signed-off-by: Mathieu Desnoyers --- liblttng-ust/ltt-events.c | 27 ++++++++++++++++++++------- liblttng-ust/ltt-probes.c | 34 ++++++++++++++++++++++++---------- liblttng-ust/tracepoint.c | 29 +++++++++++++++++++++-------- tests/demo/Makefile.am | 4 +++- tests/fork/Makefile.am | 2 +- tests/hello.cxx/Makefile.am | 2 +- tests/hello/Makefile.am | 2 +- 7 files changed, 71 insertions(+), 29 deletions(-) diff --git a/liblttng-ust/ltt-events.c b/liblttng-ust/ltt-events.c index 9650e8b0..f69eb07a 100644 --- a/liblttng-ust/ltt-events.c +++ b/liblttng-ust/ltt-events.c @@ -94,14 +94,20 @@ int add_pending_probe(struct ltt_event *event, const char *name) { struct cds_hlist_head *head; struct ust_pending_probe *e; - size_t name_len = strlen(name) + 1; - uint32_t hash = jhash(name, name_len - 1, 0); + size_t name_len = strlen(name); + uint32_t hash; + if (name_len > LTTNG_UST_SYM_NAME_LEN - 1) { + WARN("Truncating tracepoint name %s which exceeds size limits of %u chars", name, LTTNG_UST_SYM_NAME_LEN - 1); + name_len = LTTNG_UST_SYM_NAME_LEN - 1; + } + hash = jhash(name, name_len, 0); head = &pending_probe_table[hash & (PENDING_PROBE_HASH_SIZE - 1)]; e = zmalloc(sizeof(struct ust_pending_probe) + name_len); if (!e) return -ENOMEM; - memcpy(&e->name[0], name, name_len); + memcpy(&e->name[0], name, name_len + 1); + e->name[name_len] = '\0'; cds_hlist_add_head(&e->node, head); e->event = event; event->pending_probe = e; @@ -133,10 +139,10 @@ int pending_probe_fix_events(const struct lttng_event_desc *desc) struct cds_hlist_node *node, *p; struct ust_pending_probe *e; const char *name = desc->name; - size_t name_len = strlen(name) + 1; - uint32_t hash = jhash(name, name_len - 1, 0); int ret = 0; struct lttng_ust_event event_param; + size_t name_len = strlen(name); + uint32_t hash; /* * For this event, we need to lookup the loglevel. If active (in @@ -209,12 +215,17 @@ int pending_probe_fix_events(const struct lttng_event_desc *desc) } } + if (name_len > LTTNG_UST_SYM_NAME_LEN - 1) { + WARN("Truncating tracepoint name %s which exceeds size limits of %u chars", name, LTTNG_UST_SYM_NAME_LEN - 1); + name_len = LTTNG_UST_SYM_NAME_LEN - 1; + } + hash = jhash(name, name_len, 0); head = &pending_probe_table[hash & (PENDING_PROBE_HASH_SIZE - 1)]; cds_hlist_for_each_entry_safe(e, node, p, head, node) { struct ltt_event *event; struct ltt_channel *chan; - if (strcmp(name, e->name)) + if (strncmp(name, e->name, LTTNG_UST_SYM_NAME_LEN - 1)) continue; event = e->event; chan = event->chan; @@ -490,7 +501,9 @@ int ltt_event_create(struct ltt_channel *chan, * creation). Might require a hash if we have lots of events. */ cds_list_for_each_entry(event, &chan->session->events, list) { - if (event->desc && !strcmp(event->desc->name, event_param->name)) { + if (event->desc && !strncmp(event->desc->name, + event_param->name, + LTTNG_UST_SYM_NAME_LEN - 1)) { ret = -EEXIST; goto exist; } diff --git a/liblttng-ust/ltt-probes.c b/liblttng-ust/ltt-probes.c index 216ed17c..9dc23560 100644 --- a/liblttng-ust/ltt-probes.c +++ b/liblttng-ust/ltt-probes.c @@ -60,7 +60,8 @@ const struct lttng_event_desc *find_event(const char *name) cds_list_for_each_entry(probe_desc, &probe_list, head) { for (i = 0; i < probe_desc->nr_events; i++) { - if (!strcmp(probe_desc->event_desc[i]->name, name)) + if (!strncmp(probe_desc->event_desc[i]->name, name, + LTTNG_UST_SYM_NAME_LEN - 1)) return probe_desc->event_desc[i]; } } @@ -227,11 +228,17 @@ struct loglevel_entry *get_loglevel(const char *name) struct cds_hlist_head *head; struct cds_hlist_node *node; struct loglevel_entry *e; - uint32_t hash = jhash(name, strlen(name), 0); + size_t name_len = strlen(name); + uint32_t hash; + if (name_len > LTTNG_UST_SYM_NAME_LEN - 1) { + WARN("Truncating loglevel name %s which exceeds size limits of %u chars", name, LTTNG_UST_SYM_NAME_LEN - 1); + name_len = LTTNG_UST_SYM_NAME_LEN - 1; + } + hash = jhash(name, name_len, 0); head = &loglevel_table[hash & (LOGLEVEL_TABLE_SIZE - 1)]; cds_hlist_for_each_entry(e, node, head, hlist) { - if (!strcmp(name, e->name)) + if (!strncmp(name, e->name, LTTNG_UST_SYM_NAME_LEN - 1)) return e; } return NULL; @@ -274,7 +281,8 @@ void _probes_create_loglevel_events(struct loglevel_entry *entry, if (atoll(entry->name) == ev_ll->value) { match = 1; } - } else if (!strcmp(ev_ll->identifier, entry->name)) { + } else if (!strncmp(ev_ll->identifier, entry->name, + LTTNG_UST_SYM_NAME_LEN - 1)) { match = 1; } @@ -314,14 +322,19 @@ struct session_loglevel *add_loglevel(const char *name, struct cds_hlist_node *node; struct loglevel_entry *e; struct session_loglevel *sl; - size_t name_len = strlen(name) + 1; - uint32_t hash = jhash(name, name_len-1, 0); int found = 0; + size_t name_len = strlen(name); + uint32_t hash; + if (name_len > LTTNG_UST_SYM_NAME_LEN - 1) { + WARN("Truncating loglevel name %s which exceeds size limits of %u chars", name, LTTNG_UST_SYM_NAME_LEN - 1); + name_len = LTTNG_UST_SYM_NAME_LEN - 1; + } + hash = jhash(name, name_len, 0); /* loglevel entry */ head = &loglevel_table[hash & (LOGLEVEL_TABLE_SIZE - 1)]; cds_hlist_for_each_entry(e, node, head, hlist) { - if (!strcmp(name, e->name)) { + if (!strncmp(name, e->name, LTTNG_UST_SYM_NAME_LEN - 1)) { found = 1; break; } @@ -332,10 +345,11 @@ struct session_loglevel *add_loglevel(const char *name, * Using zmalloc here to allocate a variable length element. Could * cause some memory fragmentation if overused. */ - e = zmalloc(sizeof(struct loglevel_entry) + name_len); + e = zmalloc(sizeof(struct loglevel_entry) + name_len + 1); if (!e) return ERR_PTR(-ENOMEM); - memcpy(&e->name[0], name, name_len); + memcpy(&e->name[0], name, name_len + 1); + e->name[name_len] = '\0'; cds_hlist_add_head(&e->hlist, head); CDS_INIT_LIST_HEAD(&e->session_list); } @@ -512,7 +526,7 @@ struct session_wildcard *add_wildcard(const char *name, /* wildcard entry */ cds_list_for_each_entry(e, &wildcard_list, list) { - if (!strcmp(name, e->name)) { + if (!strncmp(name, e->name, LTTNG_UST_SYM_NAME_LEN - 1)) { found = 1; break; } diff --git a/liblttng-ust/tracepoint.c b/liblttng-ust/tracepoint.c index 0393aa46..f146d382 100644 --- a/liblttng-ust/tracepoint.c +++ b/liblttng-ust/tracepoint.c @@ -31,6 +31,7 @@ #include #include +#include /* for LTTNG_UST_SYM_NAME_LEN */ #include #include @@ -207,11 +208,17 @@ static struct tracepoint_entry *get_tracepoint(const char *name) struct cds_hlist_head *head; struct cds_hlist_node *node; struct tracepoint_entry *e; - uint32_t hash = jhash(name, strlen(name), 0); + size_t name_len = strlen(name); + uint32_t hash; + if (name_len > LTTNG_UST_SYM_NAME_LEN - 1) { + WARN("Truncating tracepoint name %s which exceeds size limits of %u chars", name, LTTNG_UST_SYM_NAME_LEN - 1); + name_len = LTTNG_UST_SYM_NAME_LEN - 1; + } + hash = jhash(name, name_len, 0); head = &tracepoint_table[hash & (TRACEPOINT_TABLE_SIZE - 1)]; cds_hlist_for_each_entry(e, node, head, hlist) { - if (!strcmp(name, e->name)) + if (!strncmp(name, e->name, LTTNG_UST_SYM_NAME_LEN - 1)) return e; } return NULL; @@ -226,12 +233,17 @@ static struct tracepoint_entry *add_tracepoint(const char *name) struct cds_hlist_head *head; struct cds_hlist_node *node; struct tracepoint_entry *e; - size_t name_len = strlen(name) + 1; - uint32_t hash = jhash(name, name_len-1, 0); + size_t name_len = strlen(name); + uint32_t hash; + if (name_len > LTTNG_UST_SYM_NAME_LEN - 1) { + WARN("Truncating tracepoint name %s which exceeds size limits of %u chars", name, LTTNG_UST_SYM_NAME_LEN - 1); + name_len = LTTNG_UST_SYM_NAME_LEN - 1; + } + hash = jhash(name, name_len, 0); head = &tracepoint_table[hash & (TRACEPOINT_TABLE_SIZE - 1)]; cds_hlist_for_each_entry(e, node, head, hlist) { - if (!strcmp(name, e->name)) { + if (!strncmp(name, e->name, LTTNG_UST_SYM_NAME_LEN - 1)) { DBG("tracepoint %s busy", name); return ERR_PTR(-EEXIST); /* Already there */ } @@ -240,10 +252,11 @@ static struct tracepoint_entry *add_tracepoint(const char *name) * Using zmalloc here to allocate a variable length element. Could * cause some memory fragmentation if overused. */ - e = zmalloc(sizeof(struct tracepoint_entry) + name_len); + e = zmalloc(sizeof(struct tracepoint_entry) + name_len + 1); if (!e) return ERR_PTR(-ENOMEM); - memcpy(&e->name[0], name, name_len); + memcpy(&e->name[0], name, name_len + 1); + e->name[name_len] = '\0'; e->probes = NULL; e->refcount = 0; cds_hlist_add_head(&e->hlist, head); @@ -266,7 +279,7 @@ static void remove_tracepoint(struct tracepoint_entry *e) static void set_tracepoint(struct tracepoint_entry **entry, struct tracepoint *elem, int active) { - WARN_ON(strcmp((*entry)->name, elem->name) != 0); + WARN_ON(strncmp((*entry)->name, elem->name, LTTNG_UST_SYM_NAME_LEN - 1) != 0); /* * rcu_assign_pointer has a cmm_smp_wmb() which makes sure that the new diff --git a/tests/demo/Makefile.am b/tests/demo/Makefile.am index 40e43690..6e4a603b 100644 --- a/tests/demo/Makefile.am +++ b/tests/demo/Makefile.am @@ -1,4 +1,6 @@ -AM_CPPFLAGS = -I$(top_srcdir)/include +# -Wsystem-headers is needed to print warnings in the tracepoint +# description file. +AM_CPPFLAGS = -I$(top_srcdir)/include -Wsystem-headers # We install the plugins into the /tmp/lttng-ust-divert directory to # please libtool, which absolutely needs a target install location. We diff --git a/tests/fork/Makefile.am b/tests/fork/Makefile.am index 29464f41..48b3da28 100644 --- a/tests/fork/Makefile.am +++ b/tests/fork/Makefile.am @@ -1,4 +1,4 @@ -AM_CPPFLAGS = -I$(top_srcdir)/include +AM_CPPFLAGS = -I$(top_srcdir)/include -Wsystem-headers noinst_PROGRAMS = fork fork2 fork_SOURCES = fork.c ust_tests_fork.h diff --git a/tests/hello.cxx/Makefile.am b/tests/hello.cxx/Makefile.am index 81c2a501..3e84796a 100644 --- a/tests/hello.cxx/Makefile.am +++ b/tests/hello.cxx/Makefile.am @@ -1,4 +1,4 @@ -AM_CPPFLAGS = -I$(top_srcdir)/include +AM_CPPFLAGS = -I$(top_srcdir)/include -Wsystem-headers noinst_PROGRAMS = hello hello_SOURCES = hello.cpp tp.c ust_tests_hello.h diff --git a/tests/hello/Makefile.am b/tests/hello/Makefile.am index 50b7b5e2..d866b6da 100644 --- a/tests/hello/Makefile.am +++ b/tests/hello/Makefile.am @@ -1,4 +1,4 @@ -AM_CPPFLAGS = -I$(top_srcdir)/include +AM_CPPFLAGS = -I$(top_srcdir)/include -Wsystem-headers noinst_PROGRAMS = hello hello_SOURCES = hello.c tp.c ust_tests_hello.h -- 2.34.1