[lttng-dev] [PATCH lttng-ust] Rename public structure to avoid collisions

Simon Marchi simon.marchi at polymtl.ca
Mon Sep 22 13:22:04 EDT 2014


On 13 August 2014 19:10, Simon Marchi <simon.marchi at ericsson.com> wrote:
> From: Simon Marchi <simon.marchi at polymtl.ca>
>
> 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

Ping.



More information about the lttng-dev mailing list