[lttng-dev] [RFC PATCH lttng-tools] Fix: convey enum value signedness into metadata

[Jérémie Galarneau]

Merged, thanks!

Jérémie

On Sun, Mar 20, 2016 at 2:54 PM, Mathieu Desnoyers
<mathieu.desnoyers at efficios.com> wrote:
> Currently, passing an enum range of:
>
>     ctf_enum_range("blah", 0, UINT_MAX)
>
> in LTTng-UST will print a range of 0 ... -1 in the generated CTF
> metadata, which does not reflect signedness of the values.
>
> Also, struct ustctl_enum_entry is missing a LTTNG_PACKED attribute,
> which is against our protocol rules.
>
> This change needs to be pushed in locked-step into lttng-tools and
> lttng-ust, since it breaks the protocol between the two when UST
> uses the new enumeration type (introduced in 2.8.0-rc1).
>
> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers at efficios.com>
> ---
>  src/bin/lttng-sessiond/lttng-ust-ctl.h | 11 +++++++++--
>  src/bin/lttng-sessiond/ust-metadata.c  | 28 ++++++++++++++++++++--------
>  src/bin/lttng-sessiond/ust-registry.c  | 13 +++++++++++--
>  3 files changed, 40 insertions(+), 12 deletions(-)
>
> diff --git a/src/bin/lttng-sessiond/lttng-ust-ctl.h b/src/bin/lttng-sessiond/lttng-ust-ctl.h
> index 1ea7b93..b8cf775 100644
> --- a/src/bin/lttng-sessiond/lttng-ust-ctl.h
> +++ b/src/bin/lttng-sessiond/lttng-ust-ctl.h
> @@ -286,12 +286,19 @@ struct ustctl_float_type {
>         char padding[USTCTL_UST_FLOAT_TYPE_PADDING];
>  } LTTNG_PACKED;
>
> +#define USTCTL_UST_ENUM_VALUE_PADDING  15
> +struct ustctl_enum_value {
> +       uint64_t value;
> +       uint8_t signedness;
> +       char padding[USTCTL_UST_ENUM_VALUE_PADDING];
> +} LTTNG_PACKED;
> +
>  #define USTCTL_UST_ENUM_ENTRY_PADDING  32
>  struct ustctl_enum_entry {
> -       uint64_t start, end;            /* start and end are inclusive */
> +       struct ustctl_enum_value start, end; /* start and end are inclusive */
>         char string[LTTNG_UST_SYM_NAME_LEN];
>         char padding[USTCTL_UST_ENUM_ENTRY_PADDING];
> -};
> +} LTTNG_PACKED;
>
>  #define USTCTL_UST_BASIC_TYPE_PADDING  296
>  union _ustctl_basic_type {
> diff --git a/src/bin/lttng-sessiond/ust-metadata.c b/src/bin/lttng-sessiond/ust-metadata.c
> index e49f237..170a397 100644
> --- a/src/bin/lttng-sessiond/ust-metadata.c
> +++ b/src/bin/lttng-sessiond/ust-metadata.c
> @@ -304,18 +304,30 @@ int ust_metadata_enum_statedump(struct ust_registry_session *session,
>                 if (ret) {
>                         goto end;
>                 }
> -               if (entry->start == entry->end) {
> +
> +               if (entry->start.signedness)
>                         ret = lttng_metadata_printf(session,
> -                                       "%d,\n",
> -                                       entry->start);
> -               } else {
> +                               "%lld", (long long) entry->start.value);
> +               else
>                         ret = lttng_metadata_printf(session,
> -                                       "%d ... %d,\n",
> -                                       entry->start, entry->end);
> -               }
> -               if (ret) {
> +                               "%llu", entry->start.value);
> +               if (ret)
>                         goto end;
> +               if (entry->start.signedness == entry->end.signedness &&
> +                               entry->start.value == entry->end.value) {
> +                       ret = lttng_metadata_printf(session,
> +                               ",\n");
> +               } else {
> +                       if (entry->end.signedness) {
> +                               ret = lttng_metadata_printf(session,
> +                                       " ... %lld,\n", (long long) entry->end.value);
> +                       } else {
> +                               ret = lttng_metadata_printf(session,
> +                                       " ... %llu,\n", entry->end.value);
> +                       }
>                 }
> +               if (ret)
> +                       goto end;
>         }
>         sanitize_ctf_identifier(identifier, field_name);
>         ret = print_tabs(session, nesting);
> diff --git a/src/bin/lttng-sessiond/ust-registry.c b/src/bin/lttng-sessiond/ust-registry.c
> index dea443d..6913487 100644
> --- a/src/bin/lttng-sessiond/ust-registry.c
> +++ b/src/bin/lttng-sessiond/ust-registry.c
> @@ -89,14 +89,23 @@ static int compare_enums(const struct ust_registry_enum *reg_enum_a,
>
>                 entries_a = &reg_enum_a->entries[i];
>                 entries_b = &reg_enum_b->entries[i];
> -               if (entries_a->start != entries_b->start) {
> +               if (entries_a->start.value != entries_b->start.value) {
>                         ret = -1;
>                         goto end;
>                 }
> -               if (entries_a->end != entries_b->end) {
> +               if (entries_a->end.value != entries_b->end.value) {
>                         ret = -1;
>                         goto end;
>                 }
> +               if (entries_a->start.signedness != entries_b->start.signedness) {
> +                       ret = -1;
> +                       goto end;
> +               }
> +               if (entries_a->end.signedness != entries_b->end.signedness) {
> +                       ret = -1;
> +                       goto end;
> +               }
> +
>                 if (strcmp(entries_a->string, entries_b->string)) {
>                         ret = -1;
>                         goto end;
> --
> 2.1.4
>



-- 
Jérémie Galarneau
EfficiOS Inc.
http://www.efficios.com