Fix: remove underscores from CTF IR field names at the source

The CTF spec. states that if a structure field type's field name or a
variant field type's choice name starts with `_` in the TSDL metadata
stream, then this underscore should be removed from the effective
field/choice name.

This patch applies this at the source (src.ctf plugin), in the TSDL
metadata text to CTF IR objects visitor. To do so it must also remove
such underscore characters from the components of field references (tag
and length names).

The behaviour is removed from plugins/text/pretty/print.c which did it
at the output level.

A potential issue is that a variant field type can have its enumeration
tag field type contain mappings which still have the underscore
prefixes. To circumvent this, we add the field names and the field names
with underscores to the field types's hash tables of field names to
field indexes, so that `field_name` and `_field_name` refer to the same
index within the structure/variant field type, for example.

When serializing the structure/variant field types to TSDL (for the CTF
writer API), we prefix each field with `_` if it's a CTF identifier so
as to avoid conflicts. We also remove the check that structure/variant
field type field names must not be CTF identifiers since it is safe to
serialize them now.

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/field-types.c b/lib/ctf-ir/field-types.c
index 5063ea65b87f7f8a842610d421c3923ecafb5276..653c6df25bc228d09f217c4a883fa9c7149dfaee 100644 (file)
--- a/lib/ctf-ir/field-types.c
+++ b/lib/ctf-ir/field-types.c
@@ -425,7 +425,19 @@ 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,
@@ -436,6 +448,14 @@ 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.");
@@ -449,10 +469,14 @@ 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;
 }
 
@@ -2035,14 +2059,6 @@ int bt_ctf_field_type_structure_add_field(struct bt_ctf_field_type *type,
                goto end;
        }
 
-       if (bt_ctf_validate_identifier(field_name)) {
-               BT_LOGW("Invalid parameter: field name is not a valid CTF identifier: "
-                       "struct-ft-addr=%p, field-name=\"%s\"",
-                       type, field_name);
-               ret = -1;
-               goto end;
-       }
-
        if (type->id != BT_CTF_FIELD_TYPE_ID_STRUCT) {
                BT_LOGW("Invalid parameter: field type is not a structure field type: "
                        "addr=%p, ft-id=%s", type,
@@ -2368,14 +2384,6 @@ int bt_ctf_field_type_variant_add_field(struct bt_ctf_field_type *type,
                goto end;
        }
 
-       if (bt_ctf_validate_identifier(field_name)) {
-               BT_LOGW("Invalid parameter: field name is not a valid CTF identifier: "
-                       "variant-ft-addr=%p, field-name=\"%s\"",
-                       type, field_name);
-               ret = -1;
-               goto end;
-       }
-
        if (type->id != BT_CTF_FIELD_TYPE_ID_VARIANT) {
                BT_LOGW("Invalid parameter: field type is not a variant field type: "
                        "addr=%p, ft-id=%s", type,
@@ -3862,6 +3870,19 @@ const char *get_integer_base_string(enum bt_ctf_integer_base base)
        return base_string;
 }
 
+static
+void append_struct_variant_field_name(struct metadata_context *context,
+               const char *name)
+{
+       g_string_append_c(context->string, ' ');
+
+       if (bt_ctf_validate_identifier(name)) {
+               g_string_append_c(context->string, '_');
+       }
+
+       g_string_append(context->string, name);
+}
+
 static
 int bt_ctf_field_type_integer_serialize(struct bt_ctf_field_type *type,
                struct metadata_context *context)
@@ -3959,7 +3980,7 @@ int bt_ctf_field_type_enumeration_serialize(struct bt_ctf_field_type *type,
        }
 
        if (context->field_name->len) {
-               g_string_append_printf(context->string, " %s",
+               append_struct_variant_field_name(context,
                        context->field_name->str);
                g_string_assign(context->field_name, "");
        }
@@ -4030,7 +4051,7 @@ int bt_ctf_field_type_structure_serialize(struct bt_ctf_field_type *type,
                }
 
                if (context->field_name->len) {
-                       g_string_append_printf(context->string, " %s",
+                       append_struct_variant_field_name(context,
                                context->field_name->str);
                }
                g_string_append(context->string, ";\n");
@@ -4100,8 +4121,9 @@ int bt_ctf_field_type_variant_serialize(struct bt_ctf_field_type *type,
                }
 
                if (context->field_name->len) {
-                       g_string_append_printf(context->string, " %s;",
+                       append_struct_variant_field_name(context,
                                context->field_name->str);
+                       g_string_append_c(context->string, ';');
                }
 
                g_string_append_c(context->string, '\n');
@@ -4139,8 +4161,10 @@ int bt_ctf_field_type_array_serialize(struct bt_ctf_field_type *type,
        }
 
        if (context->field_name->len) {
-               g_string_append_printf(context->string, " %s[%u]",
-                       context->field_name->str, array->length);
+               append_struct_variant_field_name(context,
+                       context->field_name->str);
+
+               g_string_append_printf(context->string, "[%u]", array->length);
                g_string_assign(context->field_name, "");
        } else {
                g_string_append_printf(context->string, "[%u]", array->length);
@@ -4168,8 +4192,10 @@ int bt_ctf_field_type_sequence_serialize(struct bt_ctf_field_type *type,
        }
 
        if (context->field_name->len) {
-               g_string_append_printf(context->string, " %s[%s]",
-                       context->field_name->str,
+               append_struct_variant_field_name(context,
+                       context->field_name->str);
+
+               g_string_append_printf(context->string, "[%s]",
                        sequence->length_field_name->str);
                g_string_assign(context->field_name, "");
        } else {

diff --git a/plugins/ctf/common/metadata/visitor-generate-ir.c b/plugins/ctf/common/metadata/visitor-generate-ir.c
index c7c6f10d3bc7e015f91949c9f2e17d4ebda9b121..34c3716ad3809c0fea89937d8789a15b70276e6b 100644 (file)
--- a/plugins/ctf/common/metadata/visitor-generate-ir.c
+++ b/plugins/ctf/common/metadata/visitor-generate-ir.c
@@ -664,6 +664,61 @@ static
 int visit_type_specifier_list(struct ctx *ctx, struct ctf_node *ts_list,
        struct bt_ctf_field_type **decl);
 
+static
+char *remove_underscores_from_field_ref(const char *field_ref)
+{
+       const char *in_ch;
+       char *out_ch;
+       char *ret;
+       enum {
+               UNDERSCORE_REMOVE_STATE_REMOVE_NEXT_UNDERSCORE,
+               UNDERSCORE_REMOVE_STATE_DO_NOT_REMOVE_NEXT_UNDERSCORE,
+       } state = UNDERSCORE_REMOVE_STATE_REMOVE_NEXT_UNDERSCORE;
+
+       assert(field_ref);
+       ret = calloc(strlen(field_ref) + 1, 1);
+       if (!ret) {
+               BT_LOGE("Failed to allocate a string: size=%zu",
+                       strlen(field_ref) + 1);
+               goto end;
+       }
+
+       in_ch = field_ref;
+       out_ch = ret;
+
+       while (*in_ch != '\0') {
+               switch (*in_ch) {
+               case ' ':
+               case '\t':
+                       /* Remove whitespace */
+                       in_ch++;
+                       continue;
+               case '_':
+                       if (state == UNDERSCORE_REMOVE_STATE_REMOVE_NEXT_UNDERSCORE) {
+                               in_ch++;
+                               state = UNDERSCORE_REMOVE_STATE_DO_NOT_REMOVE_NEXT_UNDERSCORE;
+                               continue;
+                       }
+
+                       goto copy;
+               case '.':
+                       state = UNDERSCORE_REMOVE_STATE_REMOVE_NEXT_UNDERSCORE;
+                       goto copy;
+               default:
+                       state = UNDERSCORE_REMOVE_STATE_DO_NOT_REMOVE_NEXT_UNDERSCORE;
+                       goto copy;
+               }
+
+copy:
+               *out_ch = *in_ch;
+               in_ch++;
+               out_ch++;
+       }
+
+end:
+       return ret;
+}
+
 static
 int is_unary_string(struct bt_list_head *head)
 {
@@ -1319,6 +1374,10 @@ int visit_type_declarator(struct ctx *ctx, struct ctf_node *type_specifier_list,
                        const char *id =
                                node_type_declarator->u.type_declarator.u.id;
 
+                       if (id[0] == '_') {
+                               id++;
+                       }
+
                        *field_name = g_quark_from_string(id);
                } else {
                        *field_name = 0;
@@ -1376,6 +1435,7 @@ int visit_type_declarator(struct ctx *ctx, struct ctf_node *type_specifier_list,
                        /* Lookup unsigned integer definition, create seq. */
                        _BT_CTF_FIELD_TYPE_INIT(seq_decl);
                        char *length_name = concatenate_unary_strings(length);
+                       char *length_name_no_underscore;
 
                        if (!length_name) {
                                _BT_LOGE_NODE(node_type_declarator,
@@ -1384,8 +1444,16 @@ int visit_type_declarator(struct ctx *ctx, struct ctf_node *type_specifier_list,
                                goto error;
                        }
 
+                       length_name_no_underscore =
+                               remove_underscores_from_field_ref(length_name);
+                       if (!length_name_no_underscore) {
+                               /* remove_underscores_from_field_ref() logs errors */
+                               ret = -EINVAL;
+                               goto error;
+                       }
                        seq_decl = bt_ctf_field_type_sequence_create(
-                               nested_decl, length_name);
+                               nested_decl, length_name_no_underscore);
+                       free(length_name_no_underscore);
                        g_free(length_name);
                        BT_PUT(nested_decl);
                        if (!seq_decl) {
@@ -2013,8 +2081,17 @@ int visit_variant_decl(struct ctx *ctx, const char *name,
                 * At this point, we have a fresh untagged variant; nobody
                 * else owns it. Set its tag now.
                 */
+               char *tag_no_underscore =
+                       remove_underscores_from_field_ref(tag);
+
+               if (!tag_no_underscore) {
+                       /* remove_underscores_from_field_ref() logs errors */
+                       goto error;
+               }
+
                ret = bt_ctf_field_type_variant_set_tag_name(
-                       untagged_variant_decl, tag);
+                       untagged_variant_decl, tag_no_underscore);
+               free(tag_no_underscore);
                if (ret) {
                        BT_LOGE("Cannot set variant field type's tag name: "
                                "tag-name=\"%s\"", tag);

diff --git a/plugins/text/pretty/print.c b/plugins/text/pretty/print.c
index 7234b8a485bbe4d94e4bbc3e37e6c5b3dd1a5535..6972ed5d9b5c42fbce929cd3f0ccfb834ea79ca0 100644 (file)
--- a/plugins/text/pretty/print.c
+++ b/plugins/text/pretty/print.c
@@ -47,15 +47,6 @@
 #define COLOR_EVENT_NAME       BT_COMMON_COLOR_BOLD BT_COMMON_COLOR_FG_MAGENTA
 #define COLOR_TIMESTAMP                BT_COMMON_COLOR_BOLD BT_COMMON_COLOR_FG_YELLOW
 
-static inline
-const char *rem_(const char *str)
-{
-       if (str[0] == '_')
-               return &str[1];
-       else
-               return str;
-}
-
 struct timestamp {
        int64_t real_timestamp; /* Relative to UNIX epoch. */
        uint64_t clock_value;   /* In cycles. */
@@ -939,7 +930,7 @@ enum bt_component_status print_struct_field(struct pretty_component *pretty,
                g_string_append(pretty->string, " ");
        }
        if (print_names) {
-               print_field_name_equal(pretty, rem_(field_name));
+               print_field_name_equal(pretty, field_name);
        }
        ret = print_field(pretty, field, print_names, NULL, 0);
        *nr_printed_fields += 1;
@@ -1261,7 +1252,7 @@ enum bt_component_status print_variant(struct pretty_component *pretty,
                        ret = BT_COMPONENT_STATUS_ERROR;
                        goto end;
                }
-               print_field_name_equal(pretty, rem_(tag_choice));
+               print_field_name_equal(pretty, tag_choice);
                bt_put(tag_field);
                bt_put(iter);
        }