From 891d6b550bf16672b0c3a7b35362f231d6e10fc1 Mon Sep 17 00:00:00 2001 From: Mathieu Desnoyers Date: Mon, 15 Mar 2021 22:35:29 -0400 Subject: [PATCH] Refactoring: struct lttng_enum_desc and lttng_enum_entry - Namespace these structures with lttng_ust_ prefix, - Use struct_size extensibility scheme, - Remove padding, - Use an array of pointers to structure rather than array of enumeration entries to allow easier handling of extensibility. This is achieved by using __LTTNG_COMPOUND_LITERAL(). Signed-off-by: Mathieu Desnoyers Change-Id: I2bf6e24e22c9646fc321f6c7c9ffcbd32ed22da0 --- include/lttng/ust-events.h | 52 +++++++++++++++++++-------- include/lttng/ust-tracepoint-event.h | 28 +++++++-------- include/ust-comm.h | 4 +-- liblttng-ust-comm/lttng-ust-comm.c | 10 +++--- liblttng-ust/lttng-events.c | 8 ++--- liblttng-ust/lttng-ust-dynamic-type.c | 9 ++--- liblttng-ust/ust-core.c | 2 +- liblttng-ust/ust-events-internal.h | 2 +- 8 files changed, 70 insertions(+), 45 deletions(-) diff --git a/include/lttng/ust-events.h b/include/lttng/ust-events.h index acea9910..164c82c4 100644 --- a/include/lttng/ust-events.h +++ b/include/lttng/ust-events.h @@ -83,16 +83,26 @@ enum lttng_enum_entry_options { LTTNG_ENUM_ENTRY_OPTION_IS_AUTO = 1U << 0, }; -#define LTTNG_UST_ENUM_ENTRY_PADDING 16 -struct lttng_enum_entry { +/* + * Enumeration entry description + * + * IMPORTANT: this structure is part of the ABI between the probe and + * UST. Fields need to be only added at the end, never reordered, never + * removed. + * + * The field @struct_size should be used to determine the size of the + * structure. It should be queried before using additional fields added + * at the end of the structure. + */ + +struct lttng_ust_enum_entry { + uint32_t struct_size; + struct lttng_enum_value start, end; /* start and end are inclusive */ const char *string; - union { - struct { - unsigned int options; - } LTTNG_PACKED extra; - char padding[LTTNG_UST_ENUM_ENTRY_PADDING]; - } u; + unsigned int options; + + /* End of base ABI. Fields below should be used after checking struct_size. */ }; #define __type_integer(_type, _byte_order, _base, _encoding) \ @@ -168,7 +178,7 @@ struct lttng_type { enum lttng_string_encodings encoding; } string; struct { - const struct lttng_enum_desc *desc; /* Enumeration mapping */ + const struct lttng_ust_enum_desc *desc; /* Enumeration mapping */ struct lttng_type *container_type; } enum_nestable; struct { @@ -191,12 +201,26 @@ struct lttng_type { } u; }; -#define LTTNG_UST_ENUM_TYPE_PADDING 24 -struct lttng_enum_desc { +/* + * Enumeration description + * + * IMPORTANT: this structure is part of the ABI between the probe and + * UST. Fields need to be only added at the end, never reordered, never + * removed. + * + * The field @struct_size should be used to determine the size of the + * structure. It should be queried before using additional fields added + * at the end of the structure. + */ + +struct lttng_ust_enum_desc { + uint32_t struct_size; + const char *name; - const struct lttng_enum_entry *entries; + const struct lttng_ust_enum_entry **entries; unsigned int nr_entries; - char padding[LTTNG_UST_ENUM_TYPE_PADDING]; + + /* End of base ABI. Fields below should be used after checking struct_size. */ }; /* @@ -460,7 +484,7 @@ struct lttng_ust_event_notifier { }; struct lttng_enum { - const struct lttng_enum_desc *desc; + const struct lttng_ust_enum_desc *desc; struct lttng_session *session; struct cds_list_head node; /* Enum list in session */ struct cds_hlist_node hlist; /* Session ht of enums */ diff --git a/include/lttng/ust-tracepoint-event.h b/include/lttng/ust-tracepoint-event.h index 872dfa57..e743ffda 100644 --- a/include/lttng/ust-tracepoint-event.h +++ b/include/lttng/ust-tracepoint-event.h @@ -147,7 +147,8 @@ void __event_template_proto___##_provider##___##_name(_TP_ARGS_DATA_PROTO(_args) /* Enumeration entry (single value) */ #undef ctf_enum_value #define ctf_enum_value(_string, _value) \ - { \ + __LTTNG_COMPOUND_LITERAL(struct lttng_ust_enum_entry, { \ + .struct_size = sizeof(struct lttng_ust_enum_entry), \ .start = { \ .value = lttng_is_signed_type(__typeof__(_value)) ? \ (long long) (_value) : (_value), \ @@ -159,12 +160,13 @@ void __event_template_proto___##_provider##___##_name(_TP_ARGS_DATA_PROTO(_args) .signedness = lttng_is_signed_type(__typeof__(_value)), \ }, \ .string = (_string), \ - }, + }), /* Enumeration entry (range) */ #undef ctf_enum_range #define ctf_enum_range(_string, _range_start, _range_end) \ - { \ + __LTTNG_COMPOUND_LITERAL(struct lttng_ust_enum_entry, { \ + .struct_size = sizeof(struct lttng_ust_enum_entry), \ .start = { \ .value = lttng_is_signed_type(__typeof__(_range_start)) ? \ (long long) (_range_start) : (_range_start), \ @@ -176,12 +178,13 @@ void __event_template_proto___##_provider##___##_name(_TP_ARGS_DATA_PROTO(_args) .signedness = lttng_is_signed_type(__typeof__(_range_end)), \ }, \ .string = (_string), \ - }, + }), /* Enumeration entry (automatic value; follows the rules of CTF) */ #undef ctf_enum_auto -#define ctf_enum_auto(_string) \ - { \ +#define ctf_enum_auto(_string) \ + __LTTNG_COMPOUND_LITERAL(struct lttng_ust_enum_entry, { \ + .struct_size = sizeof(struct lttng_ust_enum_entry), \ .start = { \ .value = -1ULL, \ .signedness = 0, \ @@ -191,12 +194,8 @@ void __event_template_proto___##_provider##___##_name(_TP_ARGS_DATA_PROTO(_args) .signedness = 0, \ }, \ .string = (_string), \ - .u = { \ - .extra = { \ - .options = LTTNG_ENUM_ENTRY_OPTION_IS_AUTO, \ - }, \ - }, \ - }, + .options = LTTNG_ENUM_ENTRY_OPTION_IS_AUTO, \ + }), #undef TP_ENUM_VALUES #define TP_ENUM_VALUES(...) \ @@ -204,7 +203,7 @@ void __event_template_proto___##_provider##___##_name(_TP_ARGS_DATA_PROTO(_args) #undef TRACEPOINT_ENUM #define TRACEPOINT_ENUM(_provider, _name, _values) \ - const struct lttng_enum_entry __enum_values__##_provider##_##_name[] = { \ + const struct lttng_ust_enum_entry *__enum_values__##_provider##_##_name[] = { \ _values \ ctf_enum_value("", 0) /* Dummy, 0-len array forbidden by C99. */ \ }; @@ -370,7 +369,8 @@ void __event_template_proto___##_provider##___##_name(_TP_ARGS_DATA_PROTO(_args) #undef TRACEPOINT_ENUM #define TRACEPOINT_ENUM(_provider, _name, _values) \ - static const struct lttng_enum_desc __enum_##_provider##_##_name = { \ + static const struct lttng_ust_enum_desc __enum_##_provider##_##_name = { \ + .struct_size = sizeof(struct lttng_ust_enum_desc), \ .name = #_provider "_" #_name, \ .entries = __enum_values__##_provider##_##_name, \ .nr_entries = _TP_ARRAY_SIZE(__enum_values__##_provider##_##_name) - 1, \ diff --git a/include/ust-comm.h b/include/ust-comm.h index 3d1ba372..aaf7c6ef 100644 --- a/include/ust-comm.h +++ b/include/ust-comm.h @@ -44,7 +44,7 @@ struct lttng_ust_event_field; struct lttng_ctx_field; -struct lttng_enum_entry; +struct lttng_ust_enum_entry; struct lttng_integer_type; struct lttng_session; @@ -292,7 +292,7 @@ int ustcomm_register_enum(int sock, int session_objd, /* session descriptor */ const char *enum_name, /* enum name (input) */ size_t nr_entries, /* entries */ - const struct lttng_enum_entry *entries, + const struct lttng_ust_enum_entry **entries, uint64_t *id); /* enum id (output) */ /* diff --git a/liblttng-ust-comm/lttng-ust-comm.c b/liblttng-ust-comm/lttng-ust-comm.c index e0b7d73b..656e170b 100644 --- a/liblttng-ust-comm/lttng-ust-comm.c +++ b/liblttng-ust-comm/lttng-ust-comm.c @@ -1303,7 +1303,7 @@ error_type: static int serialize_entries(struct ustctl_enum_entry **_entries, size_t nr_entries, - const struct lttng_enum_entry *lttng_entries) + const struct lttng_ust_enum_entry **lttng_entries) { struct ustctl_enum_entry *entries; int i; @@ -1314,10 +1314,10 @@ int serialize_entries(struct ustctl_enum_entry **_entries, return -ENOMEM; for (i = 0; i < nr_entries; i++) { struct ustctl_enum_entry *uentry; - const struct lttng_enum_entry *lentry; + const struct lttng_ust_enum_entry *lentry; uentry = &entries[i]; - lentry = <tng_entries[i]; + lentry = lttng_entries[i]; uentry->start.value = lentry->start.value; uentry->start.signedness = lentry->start.signedness; @@ -1326,7 +1326,7 @@ int serialize_entries(struct ustctl_enum_entry **_entries, strncpy(uentry->string, lentry->string, LTTNG_UST_ABI_SYM_NAME_LEN); uentry->string[LTTNG_UST_ABI_SYM_NAME_LEN - 1] = '\0'; - if (lentry->u.extra.options & LTTNG_ENUM_ENTRY_OPTION_IS_AUTO) { + if (lentry->options & LTTNG_ENUM_ENTRY_OPTION_IS_AUTO) { uentry->u.extra.options |= USTCTL_UST_ENUM_ENTRY_OPTION_IS_AUTO; } @@ -1523,7 +1523,7 @@ int ustcomm_register_enum(int sock, int session_objd, /* session descriptor */ const char *enum_name, /* enum name (input) */ size_t nr_entries, /* entries */ - const struct lttng_enum_entry *lttng_entries, + const struct lttng_ust_enum_entry **lttng_entries, uint64_t *id) { ssize_t len; diff --git a/liblttng-ust/lttng-events.c b/liblttng-ust/lttng-events.c index 3c78734f..762395d6 100644 --- a/liblttng-ust/lttng-events.c +++ b/liblttng-ust/lttng-events.c @@ -398,7 +398,7 @@ void lttng_enabler_destroy(struct lttng_enabler *enabler) } static -int lttng_enum_create(const struct lttng_enum_desc *desc, +int lttng_enum_create(const struct lttng_ust_enum_desc *desc, struct lttng_session *session) { const char *enum_name = desc->name; @@ -462,7 +462,7 @@ int lttng_create_enum_check(const struct lttng_type *type, switch (type->atype) { case atype_enum_nestable: { - const struct lttng_enum_desc *enum_desc; + const struct lttng_ust_enum_desc *enum_desc; int ret; enum_desc = type->u.enum_nestable.desc; @@ -476,7 +476,7 @@ int lttng_create_enum_check(const struct lttng_type *type, case atype_dynamic: { const struct lttng_ust_event_field *tag_field_generic; - const struct lttng_enum_desc *enum_desc; + const struct lttng_ust_enum_desc *enum_desc; int ret; tag_field_generic = lttng_ust_dynamic_type_tag_field(); @@ -1140,7 +1140,7 @@ void _event_enum_destroy(struct lttng_ust_event_common *event) /* Destroy enums of the current event. */ for (i = 0; i < event_recorder->parent->priv->desc->nr_fields; i++) { - const struct lttng_enum_desc *enum_desc; + const struct lttng_ust_enum_desc *enum_desc; const struct lttng_ust_event_field *field; struct lttng_enum *curr_enum; diff --git a/liblttng-ust/lttng-ust-dynamic-type.c b/liblttng-ust/lttng-ust-dynamic-type.c index 1647094f..ccda6475 100644 --- a/liblttng-ust/lttng-ust-dynamic-type.c +++ b/liblttng-ust/lttng-ust-dynamic-type.c @@ -16,7 +16,8 @@ #include #define ctf_enum_value(_string, _value) \ - { \ + __LTTNG_COMPOUND_LITERAL(struct lttng_ust_enum_entry, { \ + .struct_size = sizeof(struct lttng_ust_enum_entry), \ .start = { \ .signedness = lttng_is_signed_type(__typeof__(_value)), \ .value = lttng_is_signed_type(__typeof__(_value)) ? \ @@ -28,9 +29,9 @@ (long long) (_value) : (_value), \ }, \ .string = (_string), \ - }, + }), -static const struct lttng_enum_entry dt_enum[_NR_LTTNG_UST_DYNAMIC_TYPES] = { +static const struct lttng_ust_enum_entry *dt_enum[_NR_LTTNG_UST_DYNAMIC_TYPES] = { [LTTNG_UST_DYNAMIC_TYPE_NONE] = ctf_enum_value("_none", 0) [LTTNG_UST_DYNAMIC_TYPE_S8] = ctf_enum_value("_int8", 1) [LTTNG_UST_DYNAMIC_TYPE_S16] = ctf_enum_value("_int16", 2) @@ -45,7 +46,7 @@ static const struct lttng_enum_entry dt_enum[_NR_LTTNG_UST_DYNAMIC_TYPES] = { [LTTNG_UST_DYNAMIC_TYPE_STRING] = ctf_enum_value("_string", 11) }; -static const struct lttng_enum_desc dt_enum_desc = { +static const struct lttng_ust_enum_desc dt_enum_desc = { .name = "dynamic_type_enum", .entries = dt_enum, .nr_entries = LTTNG_ARRAY_SIZE(dt_enum), diff --git a/liblttng-ust/ust-core.c b/liblttng-ust/ust-core.c index 17fa7d1e..0b52f298 100644 --- a/liblttng-ust/ust-core.c +++ b/liblttng-ust/ust-core.c @@ -87,7 +87,7 @@ void lttng_counter_transport_unregister(struct lttng_counter_transport *transpor * Needed by comm layer. */ struct lttng_enum *lttng_ust_enum_get_from_desc(struct lttng_session *session, - const struct lttng_enum_desc *enum_desc) + const struct lttng_ust_enum_desc *enum_desc) { struct lttng_enum *_enum; struct cds_hlist_head *head; diff --git a/liblttng-ust/ust-events-internal.h b/liblttng-ust/ust-events-internal.h index ac85b711..f351e954 100644 --- a/liblttng-ust/ust-events-internal.h +++ b/liblttng-ust/ust-events-internal.h @@ -543,7 +543,7 @@ struct cds_list_head *lttng_get_probe_list_head(void); LTTNG_HIDDEN struct lttng_enum *lttng_ust_enum_get_from_desc(struct lttng_session *session, - const struct lttng_enum_desc *enum_desc); + const struct lttng_ust_enum_desc *enum_desc); LTTNG_HIDDEN int lttng_abi_create_root_handle(void); -- 2.34.1