[lttng-dev] [RFC PATCH] Rename public structure to avoid collisions
Zifei Tong
zifeitong at gmail.com
Sat Jul 26 00:19:35 EDT 2014
It reminds me how gdb workaround this issue [1]:
#define tracepoint ust_tracepoint
#include <ust/ust.h>
#undef tracepoint
[1]: http://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=blob;f=gdb/gdbserver/tracepoint.c;h=7c4b2911b00ac45db06989f5f3523a1f0a4d06a1;hb=HEAD#l6527
On Sat, Jul 26, 2014 at 6:43 AM, Simon Marchi <simon.marchi at ericsson.com> wrote:
> I am soliciting your comments about this patch, some people might not
> like it.
>
> 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 c5a09d8..e59606a 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
> lttng-dev at lists.lttng.org
> http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
More information about the lttng-dev
mailing list