[lttng-dev] [Patch LTTng-ust 4/7] Serialize the CTF named enumerations data structures for ust-comm

[Mathieu Desnoyers]

----- Original Message -----
> From: "Geneviève Bastien" <gbastien+lttng at versatic.net>
> To: lttng-dev at lists.lttng.org
> Sent: Friday, January 24, 2014 3:04:42 PM
> Subject: [lttng-dev] [Patch LTTng-ust 4/7] Serialize the CTF named	enumerations data structures for ust-comm
> 
> The CTF named enumerations are serialized to be ready to be sent to and
> received by the session daemon.
> 
> Signed-off-by: Geneviève Bastien <gbastien+lttng at versatic.net>
> ---
>  include/lttng/ust-tracepoint-event.h |   6 +-
>  liblttng-ust-comm/lttng-ust-comm.c   | 121
>  ++++++++++++++++++++++++++++++++++-
>  2 files changed, 125 insertions(+), 2 deletions(-)
> 
> diff --git a/include/lttng/ust-tracepoint-event.h
> b/include/lttng/ust-tracepoint-event.h
> index be58030..f7b9d32 100644
> --- a/include/lttng/ust-tracepoint-event.h
> +++ b/include/lttng/ust-tracepoint-event.h
> @@ -644,7 +644,11 @@ const struct lttng_event_desc
> __event_desc___##_provider##_##_name = {	       \
>  	.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,\
> +		  .nr_metadata = 0,						\

We usually put a newline after the {. The existing ".u = { .ext = { .model_emf_uri..."
was like this because it was all on one line. But since you split it into multi-lines,
please add the missing newlines.

> +		}								\

Also please add a comma (,) after the }, even if it is the last element. Even if
it is not strictly required by the compiler, it makes changes easier to follow when
the code evolves (no
- }
+ },
....) in the patches just to add the commas.

> +	},									\
>  };
>  
>  #include TRACEPOINT_INCLUDE
> diff --git a/liblttng-ust-comm/lttng-ust-comm.c
> b/liblttng-ust-comm/lttng-ust-comm.c
> index 751ad2e..1744b9f 100644
> --- a/liblttng-ust-comm/lttng-ust-comm.c
> +++ b/liblttng-ust-comm/lttng-ust-comm.c
> @@ -733,6 +733,13 @@ int serialize_basic_type(enum ustctl_abstract_types
> *uatype,
>  		break;
>  	}
>  	case atype_enum:
> +	{
> +		strncpy(ubt->enumeration.name, lbt->enumeration.name,
> +				LTTNG_UST_SYM_NAME_LEN);
> +		ubt->enumeration.name[LTTNG_UST_SYM_NAME_LEN - 1] = '\0';
> +		*uatype = ustctl_atype_enum;
> +		break;
> +	}
>  	case atype_array:
>  	case atype_sequence:
>  	default:
> @@ -750,6 +757,7 @@ int serialize_one_type(struct ustctl_type *ut, const
> struct lttng_type *lt)
>  	case atype_integer:
>  	case atype_float:
>  	case atype_string:
> +	case atype_enum:
>  		ret = serialize_basic_type(&ut->atype, lt->atype,
>  			&ut->u.basic, &lt->u.basic);
>  		if (ret)
> @@ -792,7 +800,6 @@ int serialize_one_type(struct ustctl_type *ut, const
> struct lttng_type *lt)
>  		ut->atype = ustctl_atype_sequence;
>  		break;
>  	}
> -	case atype_enum:
>  	default:
>  		return -EINVAL;
>  	}
> @@ -841,6 +848,118 @@ error_type:
>  }
>  
>  static
> +int serialize_enum(struct ustctl_enum *uenum,
> +		const struct lttng_enum *lenum)
> +{
> +	int i;
> +
> +	strncpy(uenum->name, lenum->name, LTTNG_UST_SYM_NAME_LEN);

... and set the last char to '\0'.

> +	/* Serialize the container_type */
> +	struct ustctl_integer_type *uit;

Never declare variables after code please. Variables should be right after
the beginning of a scope.

> +	const struct lttng_integer_type *lit;
> +	uit = &uenum->container_type;
> +	lit = &lenum->container_type;
> +	uit->size = lit->size;
> +	uit->signedness = lit->signedness;
> +	uit->reverse_byte_order = lit->reverse_byte_order;
> +	uit->base = lit->base;
> +	if (serialize_string_encoding(&uit->encoding, lit->encoding))
> +		return -EINVAL;
> +	uit->alignment = lit->alignment;
> +
> +	/* Serialize the entries */
> +	struct ustctl_enum_entry *entries;
> +	entries = zmalloc(lenum->len * sizeof(*entries));
> +	if (!entries)
> +		return -ENOMEM;
> +	for (i = 0; i < lenum->len; i++) {
> +		struct ustctl_enum_entry *uentry;
> +		const struct lttng_enum_entry *lentry;
> +
> +		uentry = &entries[i];
> +		lentry = &lenum->entries[i];
> +
> +		uentry->start = lentry->start;
> +		uentry->end = lentry->end;
> +		strncpy(uentry->string, lentry->string, LTTNG_UST_SYM_NAME_LEN);

... and set the last char to '\0'.

> +	}
> +	uenum->entries = entries;
> +	uenum->len = lenum->len;
> +	return 0;
> +}
> +
> +static
> +int serialize_metadata(size_t *_nr_write_metadata,

Do you mean to call this function "serialize_named_metadata" ?

serialize_metadata can lead someone to incorrectly assume this
takes care of all the metadata.

Thanks,

Mathieu

> +		struct ustctl_named_metadata **ustctl_metadata,
> +		size_t nr_metadata,
> +		const struct lttng_named_metadata *lttng_metadata)
> +{
> +	struct ustctl_named_metadata *metadata;
> +	int i, ret;
> +	size_t nr_write_metadata = 0;
> +
> +	metadata = zmalloc(nr_metadata * sizeof(*metadata));
> +	if (!metadata)
> +		return -ENOMEM;
> +
> +	for (i = 0; i < nr_metadata; i++) {
> +		struct ustctl_named_metadata *f;
> +		const struct lttng_named_metadata *lf;
> +
> +		f = &metadata[nr_write_metadata];
> +		lf = &lttng_metadata[i];
> +
> +		/* skip 'nowrite' fields */
> +		if (lf->nowrite)
> +			continue;
> +
> +		/* do the serialize here */
> +		switch (lf->mtype) {
> +		case mtype_enum:
> +		{
> +			struct ustctl_enum *ue;
> +			const struct lttng_enum *le;
> +
> +			ue = &f->u.ctf_enum;
> +			le = lf->u.ctf_enum;
> +			ret = serialize_enum(ue, le);
> +			if (ret)
> +				goto error;
> +
> +			f->mtype = ustctl_mtype_enum;
> +			break;
> +		}
> +		default:
> +			ret = -EINVAL;
> +			goto error;
> +		}
> +
> +		nr_write_metadata++;
> +	}
> +
> +	*_nr_write_metadata = nr_write_metadata;
> +	*ustctl_metadata = metadata;
> +	return 0;
> +
> +error:
> +	/* Free what has been allocated during metadata serialization */
> +	for (i = 0; i < nr_write_metadata; i++) {
> +		struct ustctl_named_metadata *m;
> +		m = &metadata[i];
> +		switch (m->mtype) {
> +		case ustctl_mtype_enum:
> +			free(m->u.ctf_enum.entries);
> +			break;
> +		default:
> +			break;
> +		}
> +	}
> +	free(metadata);
> +	return ret;
> +
> +}
> +
> +static
>  int serialize_ctx_fields(size_t *_nr_write_fields,
>  		struct ustctl_field **ustctl_fields,
>  		size_t nr_fields,
> --
> 1.8.5.3
> 
> 
> _______________________________________________
> lttng-dev mailing list
> lttng-dev at lists.lttng.org
> http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
> 

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com