[lttng-dev] [RFC PATCH] Rename public structure to avoid collisions
Simon Marchi
simon.marchi at polymtl.ca
Sat Jul 26 01:04:45 EDT 2014
Ok, so there is a way to hack around it. I agree with Pedro's comment though:
6527 /* UST puts a "struct tracepoint" in the global namespace, which
6528 conflicts with our tracepoint. Arguably, being a library, it
6529 shouldn't take ownership of such a generic name. We work around it
6530 here. */
On 26 July 2014 00:19, Zifei Tong <zifeitong at gmail.com> wrote:
> 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
>
> _______________________________________________
> 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