Since nobody complained about this, I'm merging this cleanup into master. Thanks!
Mathieu ----- Original Message ----- > From: "Simon Marchi" <[email protected]> > To: [email protected] > Sent: Wednesday, August 13, 2014 7:10:41 PM > Subject: [lttng-dev] [PATCH lttng-ust] Rename public structure to avoid > collisions > > From: Simon Marchi <[email protected]> > > Since nobody raised any concern about this, I submit the patch for > merging. You can find the original discussion here: > > http://lists.lttng.org/pipermail/lttng-dev/2014-July/023364.html > > (the rest of the message is identical to the RFC post) > > When trying to use lttng-ust with a program that defines its own struct > tracepoint, a name collision arises and compilation fails (see example > lower). > > I suggest the following renames (the problem only showed up with struct > tracepoint in my case, but let's be proactive): > > * struct tracepoint -> struct lttng_ust_tracepoint > * struct tracepoint_probe -> struct lttng_ust_tracepoint_probe > * struct tracepoint_dlopen -> struct lttng_ust_tracepoint_dlopen > > Strictly speaking, those names are part of the public API of LTTng, so I > understand that it would be a felony to rename it, since it breaks the > holy API. However, those structures are only used through some macros, > and I am not aware of any sensible use case where a user of lttng-ust > would directly reference those structures. Therefore, it should not break > any existing code. If you do have such a use case, where it would break > your code, please jump in the discussion. > > This should not break any application instrumented and built before the > change. Tested by building "hello" before and tracing it after the > change. > > Here is an example of error due to the name collision. This is when > building gdb, which I try to instrument with lttng-ust. > > In file included from /usr/local/include/lttng/tracepoint.h:28:0, > from ust_tracepoints.h:15, > from inf-ptrace.c:38: > /usr/local/include/lttng/tracepoint-types.h:32:8: error: redefinition of > ‘struct tracepoint’ > struct tracepoint { > ^ > In file included from inferior.h:37:0, > from inf-ptrace.c:22: > breakpoint.h:817:8: note: originally defined here > struct tracepoint > ^ > Makefile:1088: recipe for target 'inf-ptrace.o' failed > make: *** [inf-ptrace.o] Error 1 > --- > include/lttng/tracepoint-types.h | 10 +++---- > include/lttng/tracepoint.h | 29 +++++++++---------- > liblttng-ust/tracepoint-internal.h | 2 +- > liblttng-ust/tracepoint.c | 57 > ++++++++++++++++++++------------------ > 4 files changed, 51 insertions(+), 47 deletions(-) > > diff --git a/include/lttng/tracepoint-types.h > b/include/lttng/tracepoint-types.h > index 634f970..f54e30a 100644 > --- a/include/lttng/tracepoint-types.h > +++ b/include/lttng/tracepoint-types.h > @@ -23,19 +23,19 @@ > * SOFTWARE. > */ > > -struct tracepoint_probe { > +struct lttng_ust_tracepoint_probe { > void (*func)(void); > void *data; > }; > > -#define TRACEPOINT_PADDING 16 > -struct tracepoint { > +#define LTTNG_UST_TRACEPOINT_PADDING 16 > +struct lttng_ust_tracepoint { > const char *name; > int state; > - struct tracepoint_probe *probes; > + struct lttng_ust_tracepoint_probe *probes; > int *tracepoint_provider_ref; > const char *signature; > - char padding[TRACEPOINT_PADDING]; > + char padding[LTTNG_UST_TRACEPOINT_PADDING]; > }; > > #endif /* _LTTNG_TRACEPOINT_TYPES_H */ > diff --git a/include/lttng/tracepoint.h b/include/lttng/tracepoint.h > index 4b7473e..63759a2 100644 > --- a/include/lttng/tracepoint.h > +++ b/include/lttng/tracepoint.h > @@ -151,13 +151,13 @@ extern "C" { > * address. > */ > #define _DECLARE_TRACEPOINT(_provider, _name, ...) > \ > -extern struct tracepoint __tracepoint_##_provider##___##_name; > \ > +extern struct lttng_ust_tracepoint __tracepoint_##_provider##___##_name; > \ > static inline __attribute__((always_inline, unused)) lttng_ust_notrace > \ > void __tracepoint_cb_##_provider##___##_name(_TP_ARGS_PROTO(__VA_ARGS__)); > \ > static > \ > void __tracepoint_cb_##_provider##___##_name(_TP_ARGS_PROTO(__VA_ARGS__)) > \ > { > \ > - struct tracepoint_probe *__tp_probe; > \ > + struct lttng_ust_tracepoint_probe *__tp_probe; > \ > > \ > if (caa_unlikely(!TP_RCU_LINK_TEST())) > \ > return; > \ > @@ -166,7 +166,7 @@ void > __tracepoint_cb_##_provider##___##_name(_TP_ARGS_PROTO(__VA_ARGS__)) > \ > if (caa_unlikely(!__tp_probe)) > \ > goto end; > \ > do { > \ > - void (*__tp_cb)(void) = __tp_probe->func; > \ > + void (*__tp_cb)(void) = __tp_probe->func; > \ > void *__tp_data = __tp_probe->data; > \ > > \ > URCU_FORCE_CAST(void (*)(_TP_ARGS_DATA_PROTO(__VA_ARGS__)), > __tp_cb) \ > @@ -204,12 +204,12 @@ extern int __tracepoint_probe_unregister(const char > *name, void (*func)(void), > * tracepoint dynamic linkage handling (callbacks). Hidden visibility: > * shared across objects in a module/main executable. > */ > -struct tracepoint_dlopen { > +struct lttng_ust_tracepoint_dlopen { > void *liblttngust_handle; > > - int (*tracepoint_register_lib)(struct tracepoint * const > *tracepoints_start, > + int (*tracepoint_register_lib)(struct lttng_ust_tracepoint * const > *tracepoints_start, > int tracepoints_count); > - int (*tracepoint_unregister_lib)(struct tracepoint * const > *tracepoints_start); > + int (*tracepoint_unregister_lib)(struct lttng_ust_tracepoint * const > *tracepoints_start); > #ifndef _LGPL_SOURCE > void (*rcu_read_lock_sym_bp)(void); > void (*rcu_read_unlock_sym_bp)(void); > @@ -217,7 +217,7 @@ struct tracepoint_dlopen { > #endif > }; > > -extern struct tracepoint_dlopen tracepoint_dlopen; > +extern struct lttng_ust_tracepoint_dlopen tracepoint_dlopen; > > #if defined(TRACEPOINT_DEFINE) || defined(TRACEPOINT_CREATE_PROBES) > > @@ -230,7 +230,7 @@ int __tracepoint_registered > __attribute__((weak, visibility("hidden"))); > int __tracepoint_ptrs_registered > __attribute__((weak, visibility("hidden"))); > -struct tracepoint_dlopen tracepoint_dlopen > +struct lttng_ust_tracepoint_dlopen tracepoint_dlopen > __attribute__((weak, visibility("hidden"))); > > #ifndef _LGPL_SOURCE > @@ -312,9 +312,9 @@ __tracepoints__destroy(void) > * registering only _one_ instance of the tracepoints per shared-ojbect > * (or for the whole main program). > */ > -extern struct tracepoint * const __start___tracepoints_ptrs[] > +extern struct lttng_ust_tracepoint * const __start___tracepoints_ptrs[] > __attribute__((weak, visibility("hidden"))); > -extern struct tracepoint * const __stop___tracepoints_ptrs[] > +extern struct lttng_ust_tracepoint * const __stop___tracepoints_ptrs[] > __attribute__((weak, visibility("hidden"))); > > /* > @@ -344,7 +344,7 @@ extern struct tracepoint * const > __stop___tracepoints_ptrs[] > static const char __tp_strtab_##_provider##___##_name[] > \ > __attribute__((section("__tracepoints_strings"))) = > \ > #_provider ":" #_name; > \ > - struct tracepoint __tracepoint_##_provider##___##_name > \ > + struct lttng_ust_tracepoint __tracepoint_##_provider##___##_name > \ > __attribute__((section("__tracepoints"))) = > \ > { > \ > __tp_strtab_##_provider##___##_name, > \ > @@ -354,7 +354,8 @@ extern struct tracepoint * const > __stop___tracepoints_ptrs[] > _TP_EXTRACT_STRING(_args), > \ > { }, > \ > }; > \ > - static struct tracepoint * __tracepoint_ptr_##_provider##___##_name > \ > + static struct lttng_ust_tracepoint * > \ > + __tracepoint_ptr_##_provider##___##_name > \ > __attribute__((used, section("__tracepoints_ptrs"))) = > \ > &__tracepoint_##_provider##___##_name; > > @@ -371,11 +372,11 @@ __tracepoints__ptrs_init(void) > if (!tracepoint_dlopen.liblttngust_handle) > return; > tracepoint_dlopen.tracepoint_register_lib = > - URCU_FORCE_CAST(int (*)(struct tracepoint * const *, int), > + URCU_FORCE_CAST(int (*)(struct lttng_ust_tracepoint * const *, > int), > dlsym(tracepoint_dlopen.liblttngust_handle, > "tracepoint_register_lib")); > tracepoint_dlopen.tracepoint_unregister_lib = > - URCU_FORCE_CAST(int (*)(struct tracepoint * const *), > + URCU_FORCE_CAST(int (*)(struct lttng_ust_tracepoint * const *), > dlsym(tracepoint_dlopen.liblttngust_handle, > "tracepoint_unregister_lib")); > __tracepoint__init_urcu_sym(); > diff --git a/liblttng-ust/tracepoint-internal.h > b/liblttng-ust/tracepoint-internal.h > index 2c99dbd..2f18355 100644 > --- a/liblttng-ust/tracepoint-internal.h > +++ b/liblttng-ust/tracepoint-internal.h > @@ -27,7 +27,7 @@ > > struct tracepoint_lib { > struct cds_list_head list; /* list of registered libs */ > - struct tracepoint * const *tracepoints_start; > + struct lttng_ust_tracepoint * const *tracepoints_start; > int tracepoints_count; > struct cds_list_head callsites; > }; > diff --git a/liblttng-ust/tracepoint.c b/liblttng-ust/tracepoint.c > index 00c4719..a4a79ba 100644 > --- a/liblttng-ust/tracepoint.c > +++ b/liblttng-ust/tracepoint.c > @@ -45,7 +45,7 @@ > /* Set to 1 to enable tracepoint debug output */ > static const int tracepoint_debug; > static int initialized; > -static void (*new_tracepoint_cb)(struct tracepoint *); > +static void (*new_tracepoint_cb)(struct lttng_ust_tracepoint *); > > /* > * tracepoint_mutex nests inside UST mutex. > @@ -93,7 +93,7 @@ static int need_update; > */ > struct tracepoint_entry { > struct cds_hlist_node hlist; > - struct tracepoint_probe *probes; > + struct lttng_ust_tracepoint_probe *probes; > int refcount; /* Number of times armed. 0 if disarmed. */ > int callsite_refcount; /* how many libs use this tracepoint */ > const char *signature; > @@ -106,7 +106,7 @@ struct tp_probes { > /* Field below only used for call_rcu scheme */ > /* struct rcu_head head; */ > } u; > - struct tracepoint_probe probes[0]; > + struct lttng_ust_tracepoint_probe probes[0]; > }; > > /* > @@ -120,14 +120,15 @@ static struct cds_hlist_head > callsite_table[CALLSITE_TABLE_SIZE]; > struct callsite_entry { > struct cds_hlist_node hlist; /* hash table node */ > struct cds_list_head node; /* lib list of callsites node */ > - struct tracepoint *tp; > + struct lttng_ust_tracepoint *tp; > }; > > /* coverity[+alloc] */ > static void *allocate_probes(int count) > { > - struct tp_probes *p = zmalloc(count * sizeof(struct tracepoint_probe) > - + sizeof(struct tp_probes)); > + struct tp_probes *p = > + zmalloc(count * sizeof(struct lttng_ust_tracepoint_probe) > + + sizeof(struct tp_probes)); > return p == NULL ? NULL : p->probes; > } > > @@ -158,7 +159,7 @@ tracepoint_entry_add_probe(struct tracepoint_entry > *entry, > void (*probe)(void), void *data) > { > int nr_probes = 0; > - struct tracepoint_probe *old, *new; > + struct lttng_ust_tracepoint_probe *old, *new; > > if (!probe) { > WARN_ON(1); > @@ -178,7 +179,8 @@ tracepoint_entry_add_probe(struct tracepoint_entry > *entry, > if (new == NULL) > return ERR_PTR(-ENOMEM); > if (old) > - memcpy(new, old, nr_probes * sizeof(struct tracepoint_probe)); > + memcpy(new, old, > + nr_probes * sizeof(struct lttng_ust_tracepoint_probe)); > new[nr_probes].func = probe; > new[nr_probes].data = data; > new[nr_probes + 1].func = NULL; > @@ -193,7 +195,7 @@ tracepoint_entry_remove_probe(struct tracepoint_entry > *entry, > void (*probe)(void), void *data) > { > int nr_probes = 0, nr_del = 0, i; > - struct tracepoint_probe *old, *new; > + struct lttng_ust_tracepoint_probe *old, *new; > > old = entry->probes; > > @@ -316,7 +318,7 @@ static void remove_tracepoint(struct tracepoint_entry *e) > * Sets the probe callback corresponding to one tracepoint. > */ > static void set_tracepoint(struct tracepoint_entry **entry, > - struct tracepoint *elem, int active) > + struct lttng_ust_tracepoint *elem, int active) > { > WARN_ON(strncmp((*entry)->name, elem->name, LTTNG_UST_SYM_NAME_LEN - 1) > != > 0); > /* > @@ -354,7 +356,7 @@ static void set_tracepoint(struct tracepoint_entry > **entry, > * function insures that the original callback is not used anymore. This > insured > * by preempt_disable around the call site. > */ > -static void disable_tracepoint(struct tracepoint *elem) > +static void disable_tracepoint(struct lttng_ust_tracepoint *elem) > { > elem->state = 0; > rcu_assign_pointer(elem->probes, NULL); > @@ -364,7 +366,7 @@ static void disable_tracepoint(struct tracepoint *elem) > * Add the callsite to the callsite hash table. Must be called with > * tracepoint mutex held. > */ > -static void add_callsite(struct tracepoint_lib * lib, struct tracepoint *tp) > +static void add_callsite(struct tracepoint_lib * lib, struct > lttng_ust_tracepoint *tp) > { > struct cds_hlist_head *head; > struct callsite_entry *e; > @@ -435,7 +437,7 @@ static void tracepoint_sync_callsites(const char *name) > hash = jhash(name, name_len, 0); > head = &callsite_table[hash & (CALLSITE_TABLE_SIZE - 1)]; > cds_hlist_for_each_entry(e, node, head, hlist) { > - struct tracepoint *tp = e->tp; > + struct lttng_ust_tracepoint *tp = e->tp; > > if (strncmp(name, tp->name, LTTNG_UST_SYM_NAME_LEN - 1)) > continue; > @@ -456,10 +458,10 @@ static void tracepoint_sync_callsites(const char *name) > * Updates the probe callback corresponding to a range of tracepoints. > */ > static > -void tracepoint_update_probe_range(struct tracepoint * const *begin, > - struct tracepoint * const *end) > +void tracepoint_update_probe_range(struct lttng_ust_tracepoint * const > *begin, > + struct lttng_ust_tracepoint * const *end) > { > - struct tracepoint * const *iter; > + struct lttng_ust_tracepoint * const *iter; > struct tracepoint_entry *mark_entry; > > for (iter = begin; iter < end; iter++) { > @@ -487,9 +489,9 @@ static void lib_update_tracepoints(struct tracepoint_lib > *lib) > > static void lib_register_callsites(struct tracepoint_lib *lib) > { > - struct tracepoint * const *begin; > - struct tracepoint * const *end; > - struct tracepoint * const *iter; > + struct lttng_ust_tracepoint * const *begin; > + struct lttng_ust_tracepoint * const *end; > + struct lttng_ust_tracepoint * const *iter; > > begin = lib->tracepoints_start; > end = lib->tracepoints_start + lib->tracepoints_count; > @@ -524,18 +526,18 @@ static void tracepoint_update_probes(void) > lib_update_tracepoints(lib); > } > > -static struct tracepoint_probe * > +static struct lttng_ust_tracepoint_probe * > tracepoint_add_probe(const char *name, void (*probe)(void), void *data, > const char *signature) > { > struct tracepoint_entry *entry; > - struct tracepoint_probe *old; > + struct lttng_ust_tracepoint_probe *old; > > entry = get_tracepoint(name); > if (!entry) { > entry = add_tracepoint(name, signature); > if (IS_ERR(entry)) > - return (struct tracepoint_probe *)entry; > + return (struct lttng_ust_tracepoint_probe *)entry; > } > old = tracepoint_entry_add_probe(entry, probe, data); > if (IS_ERR(old) && !entry->refcount) > @@ -707,15 +709,16 @@ end: > pthread_mutex_unlock(&tracepoint_mutex); > } > > -void tracepoint_set_new_tracepoint_cb(void (*cb)(struct tracepoint *)) > +void tracepoint_set_new_tracepoint_cb(void (*cb)(struct lttng_ust_tracepoint > *)) > { > new_tracepoint_cb = cb; > } > > -static void new_tracepoints(struct tracepoint * const *start, struct > tracepoint * const *end) > +static void new_tracepoints(struct lttng_ust_tracepoint * const *start, > + struct lttng_ust_tracepoint * const *end) > { > if (new_tracepoint_cb) { > - struct tracepoint * const *t; > + struct lttng_ust_tracepoint * const *t; > > for (t = start; t < end; t++) { > if (*t) > @@ -724,7 +727,7 @@ static void new_tracepoints(struct tracepoint * const > *start, struct tracepoint > } > } > > -int tracepoint_register_lib(struct tracepoint * const *tracepoints_start, > +int tracepoint_register_lib(struct lttng_ust_tracepoint * const > *tracepoints_start, > int tracepoints_count) > { > struct tracepoint_lib *pl, *iter; > @@ -773,7 +776,7 @@ lib_added: > return 0; > } > > -int tracepoint_unregister_lib(struct tracepoint * const *tracepoints_start) > +int tracepoint_unregister_lib(struct lttng_ust_tracepoint * const > *tracepoints_start) > { > struct tracepoint_lib *lib; > > -- > 2.0.0 > > > _______________________________________________ > lttng-dev mailing list > [email protected] > http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev > -- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com _______________________________________________ lttng-dev mailing list [email protected] http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
