Hi, Thanks for your review.
> Why is this lttng_ust_lib_ring_buffer_config:: scoping needed in c++ ? C++ will hide enumeration defined inside class/struct. Quote from http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2011/n3242.pdf, page 158: > An enumerator declared in class scope can be referred to using the class member access > operators (::, . (dot) and -> (arrow)) [Example: > struct X { > enum direction { left=’l’, right=’r’ }; > }; > void g(X* p) { > int i; > i = p->f(left); // error: left not in scope > i = p->f(X::right); // OK > } > — end example ] > inlined precompiler directives like this make the function hard to > follow (mix of #if/#else/#endif and if/else). I agree inlined precompiler directives is bad style. Could you suggest a way to remove the mix of #if/#else/#endif and if/else? > So each field need to be listed ? We usually don't put NULL > initialization for structures that are always in zero-initialized > memory. (coding style) This is related to a known issue of g++: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55606 (Bug 55606 - sorry, unimplemented: non-trivial designated initializers not supported). g++'s 'trivial designated initializers' means no out-of-order initialization, no missing initialization (except the fields on the tail of a struct), and nested initialization should be done in the form {.foo = {.bar = 1}} instead of {.foo.bar = 1}. That's why I made such modification. > Are those changes also compatible with the LLVM c++ compiler ? Actually, clang++ have designated initializers better supported than g++. All the modification about designated initializers are not required for clang++. No need to add NULL initialization, reorder initializations or change {.foo.bar = 1} into {.foo = {.bar = 1}}. These (ugly) hacks are just to make g++ happy. Cleaned-up patch attached. Thanks. >From 36930fdfa25a6ea67de551300263dee353df43e5 Mon Sep 17 00:00:00 2001 From: Zifei Tong <[email protected]> Date: Sat, 20 Apr 2013 19:38:39 +0800 Subject: [PATCH lttng-ust] Fix: make 'hello.cxx' compile with g++ Fixes #338 Signed-off-by: Zifei Tong <[email protected]> --- include/lttng/ringbuffer-config.h | 5 +++++ include/lttng/tracepoint-event.h | 12 +++++------ include/lttng/ust-events.h | 42 +++++++++++++++++++++++------------- include/lttng/ust-tracepoint-event.h | 32 +++++++++++++++++---------- tests/hello.cxx/Makefile.am | 2 +- tests/hello.cxx/tp.c | 26 ---------------------- tests/hello.cxx/tp.cpp | 26 ++++++++++++++++++++++ 7 files changed, 86 insertions(+), 59 deletions(-) delete mode 100644 tests/hello.cxx/tp.c create mode 100644 tests/hello.cxx/tp.cpp diff --git a/include/lttng/ringbuffer-config.h b/include/lttng/ringbuffer-config.h index ca52fc7..bb01eab 100644 --- a/include/lttng/ringbuffer-config.h +++ b/include/lttng/ringbuffer-config.h @@ -347,8 +347,13 @@ int lib_ring_buffer_check_config(const struct lttng_ust_lib_ring_buffer_config * unsigned int switch_timer_interval, unsigned int read_timer_interval) { + #ifdef __cplusplus + if (config->alloc == lttng_ust_lib_ring_buffer_config::RING_BUFFER_ALLOC_GLOBAL + && config->sync == lttng_ust_lib_ring_buffer_config::RING_BUFFER_SYNC_PER_CPU + #else if (config->alloc == RING_BUFFER_ALLOC_GLOBAL && config->sync == RING_BUFFER_SYNC_PER_CPU + #endif && switch_timer_interval) return -EINVAL; return 0; diff --git a/include/lttng/tracepoint-event.h b/include/lttng/tracepoint-event.h index 077eaa0..4630788 100644 --- a/include/lttng/tracepoint-event.h +++ b/include/lttng/tracepoint-event.h @@ -20,12 +20,12 @@ * SOFTWARE. */ +#ifdef TRACEPOINT_CREATE_PROBES + #ifdef __cplusplus extern "C" { #endif -#ifdef TRACEPOINT_CREATE_PROBES - #define __tp_stringify1(x) #x #define __tp_stringify(x) __tp_stringify1(x) @@ -65,10 +65,10 @@ extern "C" { #undef TRACEPOINT_INCLUDE_FILE #undef TRACEPOINT_INCLUDE -#define TRACEPOINT_CREATE_PROBES - -#endif /* TRACEPOINT_CREATE_PROBES */ - #ifdef __cplusplus } #endif + +#define TRACEPOINT_CREATE_PROBES + +#endif /* TRACEPOINT_CREATE_PROBES */ diff --git a/include/lttng/ust-events.h b/include/lttng/ust-events.h index 749478a..e97cc1b 100644 --- a/include/lttng/ust-events.h +++ b/include/lttng/ust-events.h @@ -89,15 +89,21 @@ struct lttng_enum_entry { #define __type_integer(_type, _byte_order, _base, _encoding) \ { \ - .atype = atype_integer, \ - .u.basic.integer = \ + .atype = atype_integer, \ + .u = \ { \ - .size = sizeof(_type) * CHAR_BIT, \ - .alignment = lttng_alignof(_type) * CHAR_BIT, \ - .signedness = lttng_is_signed_type(_type), \ - .reverse_byte_order = _byte_order != BYTE_ORDER, \ - .base = _base, \ - .encoding = lttng_encode_##_encoding, \ + .basic = \ + { \ + .integer = \ + { \ + .size = sizeof(_type) * CHAR_BIT, \ + .alignment = lttng_alignof(_type) * CHAR_BIT, \ + .signedness = lttng_is_signed_type(_type), \ + .reverse_byte_order = _byte_order != BYTE_ORDER, \ + .base = _base, \ + .encoding = lttng_encode_##_encoding, \ + } \ + } \ }, \ } \ @@ -123,14 +129,20 @@ struct lttng_integer_type { #define __type_float(_type) \ { \ - .atype = atype_float, \ - .u.basic._float = \ + .atype = atype_float, \ + .u = \ { \ - .exp_dig = sizeof(_type) * CHAR_BIT \ - - _float_mant_dig(_type), \ - .mant_dig = _float_mant_dig(_type), \ - .alignment = lttng_alignof(_type) * CHAR_BIT, \ - .reverse_byte_order = BYTE_ORDER != FLOAT_WORD_ORDER, \ + .basic = \ + { \ + ._float = \ + { \ + .exp_dig = sizeof(_type) * CHAR_BIT \ + - _float_mant_dig(_type), \ + .mant_dig = _float_mant_dig(_type), \ + .alignment = lttng_alignof(_type) * CHAR_BIT, \ + .reverse_byte_order = BYTE_ORDER != FLOAT_WORD_ORDER, \ + } + } }, \ } \ diff --git a/include/lttng/ust-tracepoint-event.h b/include/lttng/ust-tracepoint-event.h index e46cc1a..3aa946d 100644 --- a/include/lttng/ust-tracepoint-event.h +++ b/include/lttng/ust-tracepoint-event.h @@ -147,11 +147,14 @@ static const char \ .type = \ { \ .atype = atype_array, \ - .u.array = \ + .u = \ { \ - .length = _length, \ - .elem_type = __type_integer(_type, BYTE_ORDER, 10, _encoding), \ - }, \ + .array = \ + { \ + .elem_type = __type_integer(_type, BYTE_ORDER, 10, _encoding), \ + .length = _length, \ + } \ + } \ }, \ .nowrite = _nowrite, \ }, @@ -164,10 +167,13 @@ static const char \ .type = \ { \ .atype = atype_sequence, \ - .u.sequence = \ + .u = \ { \ - .length_type = __type_integer(_length_type, BYTE_ORDER, 10, none), \ - .elem_type = __type_integer(_type, BYTE_ORDER, 10, _encoding), \ + .sequence = + { \ + .length_type = __type_integer(_length_type, BYTE_ORDER, 10, none), \ + .elem_type = __type_integer(_type, BYTE_ORDER, 10, _encoding), \ + }, \ }, \ }, \ .nowrite = _nowrite, \ @@ -180,7 +186,10 @@ static const char \ .type = \ { \ .atype = atype_string, \ - .u.basic.string.encoding = lttng_encode_UTF8, \ + .u = + { + .basic = { .string = { .encoding = lttng_encode_UTF8 } } \ + }, \ }, \ .nowrite = _nowrite, \ }, @@ -483,7 +492,7 @@ void __event_probe__##_provider##___##_name(_TP_ARGS_DATA_PROTO(_args)); \ static \ void __event_probe__##_provider##___##_name(_TP_ARGS_DATA_PROTO(_args)) \ { \ - struct lttng_event *__event = __tp_data; \ + struct lttng_event *__event = (struct lttng_event *) __tp_data; \ struct lttng_channel *__chan = __event->chan; \ struct lttng_ust_lib_ring_buffer_ctx __ctx; \ size_t __event_len, __event_align; \ @@ -612,13 +621,14 @@ static const char * \ __ref_model_emf_uri___##_provider##___##_name \ __attribute__((weakref ("_model_emf_uri___" #_provider "___" #_name)));\ const struct lttng_event_desc __event_desc___##_provider##_##_name = { \ - .fields = __event_fields___##_provider##___##_template, \ .name = #_provider ":" #_name, \ .probe_callback = (void (*)(void)) &__event_probe__##_provider##___##_template,\ + .ctx = NULL, \ + .fields = __event_fields___##_provider##___##_template, \ .nr_fields = _TP_ARRAY_SIZE(__event_fields___##_provider##___##_template), \ .loglevel = &__ref_loglevel___##_provider##___##_name, \ .signature = __tp_event_signature___##_provider##___##_template, \ - .u.ext.model_emf_uri = &__ref_model_emf_uri___##_provider##___##_name, \ + .u = { .ext = { .model_emf_uri = &__ref_model_emf_uri___##_provider##___##_name } }, \ }; #include TRACEPOINT_INCLUDE diff --git a/tests/hello.cxx/Makefile.am b/tests/hello.cxx/Makefile.am index 897416d..5f6c615 100644 --- a/tests/hello.cxx/Makefile.am +++ b/tests/hello.cxx/Makefile.am @@ -1,7 +1,7 @@ AM_CPPFLAGS = -I$(top_srcdir)/include -I$(top_builddir)/include -Wsystem-headers noinst_PROGRAMS = hello -hello_SOURCES = hello.cpp tp.c ust_tests_hello.h +hello_SOURCES = hello.cpp tp.cpp ust_tests_hello.h hello_LDADD = $(top_builddir)/liblttng-ust/liblttng-ust.la if LTTNG_UST_BUILD_WITH_LIBDL diff --git a/tests/hello.cxx/tp.c b/tests/hello.cxx/tp.c deleted file mode 100644 index 4790965..0000000 --- a/tests/hello.cxx/tp.c +++ /dev/null @@ -1,26 +0,0 @@ -/* - * tp.c - * - * Copyright (c) 2011 Mathieu Desnoyers <[email protected]> - * - * Permission is hereby granted, free of charge, to any person obtaining a copy - * of this software and associated documentation files (the "Software"), to deal - * in the Software without restriction, including without limitation the rights - * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell - * copies of the Software, and to permit persons to whom the Software is - * furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice shall be included in - * all copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE - * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE - * SOFTWARE. - */ - -#define TRACEPOINT_CREATE_PROBES -#include "ust_tests_hello.h" diff --git a/tests/hello.cxx/tp.cpp b/tests/hello.cxx/tp.cpp new file mode 100644 index 0000000..a2099ab --- /dev/null +++ b/tests/hello.cxx/tp.cpp @@ -0,0 +1,26 @@ +/* + * tp.cpp + * + * Copyright (c) 2011 Mathieu Desnoyers <[email protected]> + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +#define TRACEPOINT_CREATE_PROBES +#include "ust_tests_hello.h" -- 1.8.2.1 -- Best Regards, 仝子飞 (Zifei Tong) College of Computer Science and Technology, Zhejiang University [email protected] / [email protected]
_______________________________________________ lttng-dev mailing list [email protected] http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
