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

Simon Marchi simon.marchi at ericsson.com
Fri Jul 25 18:43:47 EDT 2014


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




More information about the lttng-dev mailing list