[lttng-dev] [PATCH] Define tracepoint callback only if TRACEPOINT_DEFINE defined

[Mathieu Desnoyers]

----- Original Message -----
> From: "Zifei Tong" <zifeitong at gmail.com>
> To: "mathieu desnoyers" <mathieu.desnoyers at efficios.com>
> Cc: lttng-dev at lists.lttng.org, "Zifei Tong" <zifeitong at gmail.com>
> Sent: Wednesday, March 12, 2014 5:40:37 AM
> Subject: [PATCH] Define tracepoint callback only if TRACEPOINT_DEFINE defined
> 
> Compiling tracepoint providers in clang will result in 'Wunused-function'
> warning, since tracepoint callbacks are defined but never called.
> 
> lttng-gen-tp is also updated to not put TRACEPOINT_DEFINE in generated
> headers.
> 
> Fixes #760
> 
> Signed-off-by: Zifei Tong <zifeitong at gmail.com>
> ---
>  doc/examples/gen-tp/sample.c |  1 +
>  include/lttng/tracepoint.h   | 13 ++++++++++---
>  tools/lttng-gen-tp           |  1 -
>  3 files changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/doc/examples/gen-tp/sample.c b/doc/examples/gen-tp/sample.c
> index dab6e7d..ae22f28 100644
> --- a/doc/examples/gen-tp/sample.c
> +++ b/doc/examples/gen-tp/sample.c
> @@ -23,6 +23,7 @@
>  
>  #include <unistd.h>
>  
> +#define TRACEPOINT_DEFINE

Hrm, so this requires end users to change the way to use lttng-gen-tp.
I'm not sure I like that at all. This means changes in the way people do
instrumentation, and I'm trying really hard not to break that.

>  #include "sample_tracepoint.h"
>  int main(int argc, char **argv)
>  {
> diff --git a/include/lttng/tracepoint.h b/include/lttng/tracepoint.h
> index 66e2abd..462a0fc 100644
> --- a/include/lttng/tracepoint.h
> +++ b/include/lttng/tracepoint.h
> @@ -150,8 +150,8 @@ extern "C" {
>   * between caller's ip addresses within the probe using the return
>   * address.
>   */
> -#define _DECLARE_TRACEPOINT(_provider, _name, ...)			 		\
> -extern struct tracepoint __tracepoint_##_provider##___##_name;				\
> +#if defined(TRACEPOINT_DEFINE)

So you are adding a ifdef conditional to skip creation of a macro that defines
static inline functions. What is it achieving really other than silencing a compiler
warning ? Anyway the compiler will optimize those static inline away if they are
not used.

I think this is a case where we might want to use __attribute__((unused)) to just
shut up the clang warning.

Thoughts ?

Thanks,

Mathieu

> +#define _DECLARE_TRACEPOINT_CB(_provider, _name, ...)			 		\
>  static inline __attribute__((always_inline)) lttng_ust_notrace				\
>  void __tracepoint_cb_##_provider##___##_name(_TP_ARGS_PROTO(__VA_ARGS__));
>  		\
>  static											\
> @@ -174,7 +174,14 @@ void
> __tracepoint_cb_##_provider##___##_name(_TP_ARGS_PROTO(__VA_ARGS__))		\
>  	} while ((++__tp_probe)->func);							\
>  end:											\
>  	tp_rcu_read_unlock_bp();							\
> -}											\
> +}
> +#else
> +#define _DECLARE_TRACEPOINT_CB(_provider, _name, ...)
> +#endif
> +
> +#define _DECLARE_TRACEPOINT(_provider, _name, ...)			 		\
> +extern struct tracepoint __tracepoint_##_provider##___##_name;				\
> +_DECLARE_TRACEPOINT_CB(_provider, _name, __VA_ARGS__)			 		\
>  static inline lttng_ust_notrace								\
>  void __tracepoint_register_##_provider##___##_name(char *name,				\
>  		void (*func)(void), void *data);					\
> diff --git a/tools/lttng-gen-tp b/tools/lttng-gen-tp
> index c49e8a5..ff8c22d 100755
> --- a/tools/lttng-gen-tp
> +++ b/tools/lttng-gen-tp
> @@ -69,7 +69,6 @@ class CFile:
>  /*
>   * The header containing our TRACEPOINT_EVENTs.
>   */
> -#define TRACEPOINT_DEFINE
>  #include "{headerFilename}"
>  """
>      def __init__(self, filename, template):
> --
> 1.9.0
> 
> 

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com