ctf, ir: escape and unescape enumeration FT labels starting with `_`

This is a solution to work with TSDL 1.8 which asks identifiers starting
with `_` to be named without the `_` for viewing and analysis purposes.
Since a variant FT can refer to an enumeration FT tag which also has its
labels starting with `_`, we systematically remove leading `_` in
enumeration FT labels too.

For example:

    enum {
        _salut,
        hello,
        __zoom,
        _integer,
    } tag;

    variant <tag> {
        A _salut;
        B hello;
        C __zoom;
        D _integer;
    } var;

Once in CTF IR, the equivalent is:

    enum {
        salut,
        hello,
        _zoom,
        integer,
    } tag;

    variant <tag> {
        A salut;
        B hello;
        C _zoom;
        D integer;
    } var;

Once back to TSDL (through CTF writer), it is:

    enum {
        salut,
        hello,
        __zoom,
        _integer,
    } tag;

    variant <tag> {
        A salut;
        B hello;
        C __zoom;
        D _integer;
    } var;

Signed-off-by: Philippe Proulx <eeppeliteloop@gmail.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>

diff --git a/lib/ctf-ir/clock-class.c b/lib/ctf-ir/clock-class.c
index 5d623c47ddeea113628ed2f3ae3a9c76b5ab59b2..7c28fee843f6271daf4b5c77124d5c8ea9d8ab6f 100644 (file)
--- a/lib/ctf-ir/clock-class.c
+++ b/lib/ctf-ir/clock-class.c
@@ -67,7 +67,7 @@ int bt_clock_class_set_name(struct bt_clock_class *clock_class,
                goto end;
        }
 
-       if (bt_identifier_is_valid(name)) {
+       if (!bt_identifier_is_valid(name)) {
                BT_LOGE("Clock class's name is not a valid CTF identifier: "
                        "addr=%p, name=\"%s\"",
                        clock_class, name);

diff --git a/lib/ctf-ir/event-class.c b/lib/ctf-ir/event-class.c
index 4c969978b27220fbd54f225d99cdf232ba9f9504..8ff8bc0394ed9f5f2b69d62ba74d219f7c5e5747 100644 (file)
--- a/lib/ctf-ir/event-class.c
+++ b/lib/ctf-ir/event-class.c
@@ -381,7 +381,7 @@ int bt_event_class_add_field(struct bt_event_class *event_class,
                goto end;
        }
 
-       if (bt_identifier_is_valid(name)) {
+       if (!bt_identifier_is_valid(name)) {
                BT_LOGW("Invalid parameter: event class's payload field type's field name is not a valid CTF identifier: "
                        "addr=%p, name=\"%s\", id=%" PRId64 ", field-name=\"%s\"",
                        event_class, bt_event_class_get_name(event_class),

diff --git a/lib/ctf-ir/field-types.c b/lib/ctf-ir/field-types.c
index 7371e5bfcf5bb483e30bb13590524f771ff69dd1..a6ebdb391d8f9591bb92d19f749a7ba1191e71a7 100644 (file)
--- a/lib/ctf-ir/field-types.c
+++ b/lib/ctf-ir/field-types.c
@@ -425,19 +425,7 @@ int add_structure_field(GPtrArray *fields,
 {
        int ret = 0;
        GQuark name_quark = g_quark_from_string(field_name);
-       GQuark underscore_name_quark;
        struct structure_field *field;
-       GString *underscore_name = g_string_new(NULL);
-
-       if (!underscore_name) {
-               BT_LOGE_STR("Failed to allocate a GString.");
-               ret = -1;
-               goto end;
-       }
-
-       g_string_assign(underscore_name, "_");
-       g_string_append(underscore_name, field_name);
-       underscore_name_quark = g_quark_from_string(underscore_name->str);
 
        /* Make sure structure does not contain a field of the same name */
        if (g_hash_table_lookup_extended(field_name_to_index,
@@ -448,14 +436,6 @@ int add_structure_field(GPtrArray *fields,
                goto end;
        }
 
-       if (g_hash_table_lookup_extended(field_name_to_index,
-                       GUINT_TO_POINTER(underscore_name_quark), NULL, NULL)) {
-               BT_LOGW("Structure or variant field type already contains a field type with this name: "
-                       "field-name=\"%s\"", underscore_name->str);
-               ret = -1;
-               goto end;
-       }
-
        field = g_new0(struct structure_field, 1);
        if (!field) {
                BT_LOGE_STR("Failed to allocate one structure/variant field type field.");
@@ -469,14 +449,10 @@ int add_structure_field(GPtrArray *fields,
        g_hash_table_insert(field_name_to_index,
                GUINT_TO_POINTER(name_quark),
                GUINT_TO_POINTER(fields->len));
-       g_hash_table_insert(field_name_to_index,
-               GUINT_TO_POINTER(underscore_name_quark),
-               GUINT_TO_POINTER(fields->len));
        g_ptr_array_add(fields, field);
        BT_LOGV("Added structure/variant field type field: field-ft-addr=%p, "
                "field-name=\"%s\"", field_type, field_name);
 end:
-       g_string_free(underscore_name, TRUE);
        return ret;
 }
 
@@ -2220,7 +2196,7 @@ struct bt_field_type *bt_field_type_variant_create(
                "tag-ft-addr=%p, tag-field-name=\"%s\"",
                enum_tag, tag_name);
 
-       if (tag_name && bt_identifier_is_valid(tag_name)) {
+       if (tag_name && !bt_identifier_is_valid(tag_name)) {
                BT_LOGW("Invalid parameter: tag field name is not a valid CTF identifier: "
                        "tag-ft-addr=%p, tag-field-name=\"%s\"",
                        enum_tag, tag_name);
@@ -2343,7 +2319,7 @@ int bt_field_type_variant_set_tag_name(
                goto end;
        }
 
-       if (bt_identifier_is_valid(name)) {
+       if (!bt_identifier_is_valid(name)) {
                BT_LOGW("Invalid parameter: tag field name is not a valid CTF identifier: "
                        "variant-ft-addr=%p, tag-field-name=\"%s\"",
                        type, name);
@@ -2756,7 +2732,7 @@ struct bt_field_type *bt_field_type_sequence_create(
                goto error;
        }
 
-       if (bt_identifier_is_valid(length_field_name)) {
+       if (!bt_identifier_is_valid(length_field_name)) {
                BT_LOGW("Invalid parameter: length field name is not a valid CTF identifier: "
                        "length-field-name=\"%s\"", length_field_name);
                goto error;
@@ -3877,7 +3853,7 @@ void append_field_name(struct metadata_context *context,
 {
        g_string_append_c(context->string, ' ');
 
-       if (bt_identifier_is_valid(name) || *name == '_') {
+       if (!bt_identifier_is_valid(name) || *name == '_') {
                g_string_append_c(context->string, '_');
        }
 
@@ -3944,18 +3920,25 @@ int bt_field_type_enumeration_serialize(struct bt_field_type *type,
        for (entry = 0; entry < enumeration->entries->len; entry++) {
                struct enumeration_mapping *mapping =
                        enumeration->entries->pdata[entry];
+               const char *label = g_quark_to_string(mapping->string);
+
+               g_string_append(context->string, "\"");
+
+               if (!bt_identifier_is_valid(label) || label[0] == '_') {
+                       g_string_append(context->string, "_");
+               }
+
+               g_string_append_printf(context->string, "%s\" = ", label);
 
                if (container_signed) {
                        if (mapping->range_start._signed ==
                                mapping->range_end._signed) {
                                g_string_append_printf(context->string,
-                                       "\"%s\" = %" PRId64,
-                                       g_quark_to_string(mapping->string),
+                                       "%" PRId64,
                                        mapping->range_start._signed);
                        } else {
                                g_string_append_printf(context->string,
-                                       "\"%s\" = %" PRId64 " ... %" PRId64,
-                                       g_quark_to_string(mapping->string),
+                                       "%" PRId64 " ... %" PRId64,
                                        mapping->range_start._signed,
                                        mapping->range_end._signed);
                        }
@@ -3963,13 +3946,11 @@ int bt_field_type_enumeration_serialize(struct bt_field_type *type,
                        if (mapping->range_start._unsigned ==
                                mapping->range_end._unsigned) {
                                g_string_append_printf(context->string,
-                                       "\"%s\" = %" PRIu64,
-                                       g_quark_to_string(mapping->string),
+                                       "%" PRIu64,
                                        mapping->range_start._unsigned);
                        } else {
                                g_string_append_printf(context->string,
-                                       "\"%s\" = %" PRIu64 " ... %" PRIu64,
-                                       g_quark_to_string(mapping->string),
+                                       "%" PRIu64 " ... %" PRIu64,
                                        mapping->range_start._unsigned,
                                        mapping->range_end._unsigned);
                        }

diff --git a/lib/ctf-ir/trace.c b/lib/ctf-ir/trace.c
index 4a3027b48ec76517c055b5312e59f91c185a2bfa..e21bf0eb4ef03485fbcda336dbd725b60729e534 100644 (file)
--- a/lib/ctf-ir/trace.c
+++ b/lib/ctf-ir/trace.c
@@ -350,7 +350,7 @@ int bt_trace_set_environment_field(struct bt_trace *trace,
                goto end;
        }
 
-       if (bt_identifier_is_valid(name)) {
+       if (!bt_identifier_is_valid(name)) {
                BT_LOGW("Invalid parameter: environment field's name is not a valid CTF identifier: "
                        "trace-addr=%p, trace-name=\"%s\", "
                        "env-name=\"%s\"",

diff --git a/lib/ctf-ir/utils.c b/lib/ctf-ir/utils.c
index 12daedbd9105d787c35d5b8ea262c175ebfbdc55..9eff0508b31c01ba3e8b228ce69a86f844126a47 100644 (file)
--- a/lib/ctf-ir/utils.c
+++ b/lib/ctf-ir/utils.c
@@ -120,5 +120,5 @@ end:
 
 bt_bool bt_identifier_is_valid(const char *identifier)
 {
-       return bt_validate_identifier(identifier);
+       return bt_validate_identifier(identifier) == 0;
 }

diff --git a/plugins/ctf/common/metadata/visitor-generate-ir.c b/plugins/ctf/common/metadata/visitor-generate-ir.c
index f5fab1520f1c062e9b737cf4936d590924bb4e77..aae7322bc8b21069c442ea99805c605f0bb693a7 100644 (file)
--- a/plugins/ctf/common/metadata/visitor-generate-ir.c
+++ b/plugins/ctf/common/metadata/visitor-generate-ir.c
@@ -2122,6 +2122,7 @@ int visit_enum_decl_entry(struct ctx *ctx, struct ctf_node *enumerator,
        struct ctf_node *iter;
        int64_t start = 0, end = 0;
        const char *label = enumerator->u.enumerator.id;
+       const char *effective_label = label;
        struct bt_list_head *values = &enumerator->u.enumerator.values;
 
        bt_list_for_each_entry(iter, values, siblings) {
@@ -2181,12 +2182,24 @@ int visit_enum_decl_entry(struct ctx *ctx, struct ctf_node *enumerator,
 
        *last = end + 1;
 
+       if (label[0] == '_') {
+               /*
+                * Strip the first underscore of any enumeration field
+                * type's label in case this enumeration FT is used as
+                * a variant FT tag later. The variant FT choice names
+                * could also start with `_`, in which case the prefix
+                * is removed, and it the resulting choice name needs to
+                * match tag labels.
+                */
+               effective_label = &label[1];
+       }
+
        if (is_signed) {
-               ret = bt_ctf_field_type_enumeration_add_mapping(enum_decl, label,
-                       start, end);
+               ret = bt_field_type_enumeration_add_mapping_signed(enum_decl,
+                       effective_label, start, end);
        } else {
                ret = bt_field_type_enumeration_add_mapping_unsigned(enum_decl,
-                       label, (uint64_t) start, (uint64_t) end);
+                       effective_label, (uint64_t) start, (uint64_t) end);
        }
        if (ret) {
                _BT_LOGE_NODE(enumerator,