diff mbox series

[lttng-tools,1/5] Fix: probes should not be compared by their names and callsite signatures

Message ID 1518032219-28072-2-git-send-email-francis.deslauriers@efficios.com
State Superseded, archived
Headers show
Series Support probes with the same name but different event payload | expand

Commit Message

Francis Deslauriers Feb. 7, 2018, 7:36 p.m. UTC
Events with different payloads but identical name and signatures could
lead to corrupted trace as the Session Daemon would consider them
identical and give them the same event ID.

Events should be compared using the name, loglevel, fields and
model_emf_uri to ensure that their serialized layout is the same.

Signed-off-by: Francis Deslauriers <francis.deslauriers at efficios.com>
---
 src/bin/lttng-sessiond/Makefile.am       |   3 +-
 src/bin/lttng-sessiond/ust-field-utils.c | 261 +++++++++++++++++++++++++++++++
 src/bin/lttng-sessiond/ust-field-utils.h |  29 ++++
 src/bin/lttng-sessiond/ust-registry.c    |  40 ++++-
 tests/unit/Makefile.am                   |   1 +
 5 files changed, 325 insertions(+), 9 deletions(-)
 create mode 100644 src/bin/lttng-sessiond/ust-field-utils.c
 create mode 100644 src/bin/lttng-sessiond/ust-field-utils.h

Comments

Mathieu Desnoyers Feb. 7, 2018, 8:43 p.m. UTC | #1
----- On Feb 7, 2018, at 2:36 PM, Francis Deslauriers francis.deslauriers at efficios.com wrote:

> Events with different payloads but identical name and signatures could
> lead to corrupted trace as the Session Daemon would consider them
> identical and give them the same event ID.
> 
> Events should be compared using the name, loglevel, fields and
> model_emf_uri to ensure that their serialized layout is the same.

model_emf_uri has no impact on the serialized layout, but does have an
impact on what ends up in the metadata description for the event. The
changelog should be adapated to clarify this.

> 
> Signed-off-by: Francis Deslauriers <francis.deslauriers at efficios.com>
> ---
> src/bin/lttng-sessiond/Makefile.am       |   3 +-
> src/bin/lttng-sessiond/ust-field-utils.c | 261 +++++++++++++++++++++++++++++++
> src/bin/lttng-sessiond/ust-field-utils.h |  29 ++++
> src/bin/lttng-sessiond/ust-registry.c    |  40 ++++-
> tests/unit/Makefile.am                   |   1 +
> 5 files changed, 325 insertions(+), 9 deletions(-)
> create mode 100644 src/bin/lttng-sessiond/ust-field-utils.c
> create mode 100644 src/bin/lttng-sessiond/ust-field-utils.h
> 
> diff --git a/src/bin/lttng-sessiond/Makefile.am
> b/src/bin/lttng-sessiond/Makefile.am
> index 413fe75..6fc1809 100644
> --- a/src/bin/lttng-sessiond/Makefile.am
> +++ b/src/bin/lttng-sessiond/Makefile.am
> @@ -40,7 +40,8 @@ lttng_sessiond_SOURCES = utils.c utils.h \
> if HAVE_LIBLTTNG_UST_CTL
> lttng_sessiond_SOURCES += trace-ust.c ust-registry.c ust-app.c \
> 			ust-consumer.c ust-consumer.h ust-thread.c \
> -			ust-metadata.c ust-clock.h agent-thread.c agent-thread.h
> +			ust-metadata.c ust-clock.h agent-thread.c agent-thread.h \
> +			ust-field-utils.h ust-field-utils.c
> endif
> 
> # Add main.c at the end for compile order
> diff --git a/src/bin/lttng-sessiond/ust-field-utils.c
> b/src/bin/lttng-sessiond/ust-field-utils.c
> new file mode 100644
> index 0000000..3c2da14
> --- /dev/null
> +++ b/src/bin/lttng-sessiond/ust-field-utils.c
> @@ -0,0 +1,261 @@
> +/*
> + * Copyright (C) 2018 - Francis Deslauriers <francis.deslauriers at efficios.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License, version 2 only, as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program; if not, write to the Free Software Foundation, Inc., 51
> + * Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
> + */
> +
> +#include <string.h>
> +#include <stdbool.h>
> +
> +#include "ust-field-utils.h"
> +
> +/*
> + * The ustctl_field is made of a combinaison of C basic types

combination

> + * ustctl_basic_type and _ustctl_basic_type.
> + *
> + * ustctl_basic_type contains an enumeration describing the abstract type.
> + * _ustctl_basic_type does _NOT_ contain an enumeration describing the
> + * abstract type.
> + *
> + * A layer is needed to use the same code for both structures.
> + * When dealing with _ustctl_basic_type, we need to use the abstract type of
> + * the ustctl_type struct.
> + */
> +
> +/*
> + * Compare two ustctl_integer_type fields.
> + * Returns 1 if both are identical.
> + */
> +static bool match_ustctl_field_integer(struct ustctl_integer_type *first,
> +			struct ustctl_integer_type *second)
> +{
> +	bool match = true;

I don't like bitwise operators on C bool type. Is there anything that
guarantees us that bool always uses the same bit to represent "true" ?

Also, missing whiteline after the variable declaration.


> +	match &= first->size == second->size;
> +	match &= first->alignment == second->alignment;
> +	match &= first->signedness == second->signedness;
> +	match &= first->encoding == second->encoding;
> +	match &= first->base == second->base;
> +	match &= first->reverse_byte_order == second->reverse_byte_order;
> +
> +	return match;


I'd prefer simply:

if (first->size != second->size)
   return false;
.....
return true;

> +}
> +
> +/*
> + * Compare two _ustctl_basic_type fields known to be of type integer.
> + * Returns 1 if both are identical.
> + */
> +static bool match_ustctl_field_integer_from_raw_basic_type(
> +			union _ustctl_basic_type *first, union _ustctl_basic_type *second)
> +{
> +	return match_ustctl_field_integer(&first->integer, &second->integer);
> +}
> +
> +/*
> + * Compare two _ustctl_basic_type fields known to be of type enum.
> + * Returns 1 if both are identical.
> + */
> +static bool match_ustctl_field_enum_from_raw_basic_type(
> +			union _ustctl_basic_type *first, union _ustctl_basic_type *second)
> +{
> +	/*
> +	 * Compare enumeration ID. Enumeration ID is provided to the application by
> +	 * the session daemon before event registration.
> +	 */
> +	if (first->enumeration.id != second->enumeration.id) {
> +		goto no_match;
> +	}
> +
> +	/*
> +	 * Sanity check of the name and container type. Those were already checked
> +	 * during enum registration.
> +	 */
> +	if (strncmp(first->enumeration.name, second->enumeration.name,
> +				LTTNG_UST_SYM_NAME_LEN)) {
> +		goto no_match;
> +	}
> +	if (match_ustctl_field_integer(&first->enumeration.container_type,
> +				&second->enumeration.container_type) == false) {

replace if (match_...() == false) {

by

if (!match...()) {

> +		goto no_match;
> +	}
> +
> +	return true;
> +
> +no_match:
> +	return false;
> +}
> +
> +/*
> + * Compare two _ustctl_basic_type fields known to be of type string.
> + * Returns 1 if both are identical.
> + */
> +static bool match_ustctl_field_string_from_raw_basic_type(
> +			union _ustctl_basic_type *first, union _ustctl_basic_type *second)
> +{
> +	return first->string.encoding == second->string.encoding;
> +}
> +
> +/*
> + * Compare two _ustctl_basic_type fields known to be of type float.
> + * Returns 1 if both are identical.
> + */
> +static bool match_ustctl_field_float_from_raw_basic_type(
> +			union _ustctl_basic_type *first, union _ustctl_basic_type *second)
> +{
> +	bool match = true;


same here about bitwise ops and whiteline.

> +	match &= first->_float.exp_dig == second->_float.exp_dig;
> +	match &= first->_float.mant_dig == second->_float.mant_dig;
> +	match &= first->_float.reverse_byte_order ==
> second->_float.reverse_byte_order;
> +	match &= first->_float.alignment == second->_float.alignment;
> +
> +	return match;
> +}
> +
> +/*
> + * Compare two _ustctl_basic_type fields given their respective abstract types.
> + * Returns 1 if both are identical.
> + */
> +static bool match_ustctl_field_raw_basic_type(
> +			enum ustctl_abstract_types first_atype,
> +			union _ustctl_basic_type *first,
> +			enum ustctl_abstract_types second_atype,
> +			union _ustctl_basic_type *second)
> +{
> +	if (first_atype != second_atype) {
> +		goto no_match;
> +	}
> +
> +	switch (first_atype) {
> +	case ustctl_atype_integer:
> +		if (match_ustctl_field_integer_from_raw_basic_type(first, second) == false) {
> +			goto no_match;
> +		}
> +		break;
> +	case ustctl_atype_enum:
> +		if (match_ustctl_field_enum_from_raw_basic_type(first, second) == false) {
> +			goto no_match;
> +		}
> +		break;
> +	case ustctl_atype_string:
> +		if (match_ustctl_field_string_from_raw_basic_type(first, second) == false) {
> +			goto no_match;
> +		}
> +		break;
> +	case ustctl_atype_float:
> +		if (match_ustctl_field_float_from_raw_basic_type(first, second) == false) {
> +			goto no_match;
> +		}
> +		break;
> +	default:
> +		goto no_match;
> +	}
> +
> +	return true;
> +
> +no_match:
> +	return false;
> +}
> +
> +/*
> + * Compatibility layer between the ustctl_basic_type struct and
> + * _ustctl_basic_type union.
> + */
> +static bool match_ustctl_field_basic_type(struct ustctl_basic_type *first,
> +			struct ustctl_basic_type *second)
> +{
> +	return match_ustctl_field_raw_basic_type(first->atype, &first->u.basic,
> +				second->atype, &second->u.basic);
> +}
> +
> +int match_ustctl_field(struct ustctl_field *first, struct ustctl_field *second)
> +{
> +	/* Check the name of the field is identical. */
> +	if (strncmp(first->name, second->name, LTTNG_UST_SYM_NAME_LEN)) {
> +		goto no_match;
> +	}
> +
> +	/* Check the field type is identical. */
> +	if (first->type.atype != second->type.atype) {
> +		goto no_match;
> +	}
> +
> +	/* Check the field layout. */
> +	switch (first->type.atype) {
> +	case ustctl_atype_integer:
> +	case ustctl_atype_enum:
> +	case ustctl_atype_string:
> +	case ustctl_atype_float:
> +		if (match_ustctl_field_raw_basic_type(first->type.atype,
> +					&first->type.u.basic, second->type.atype,
> +					&second->type.u.basic) == false) {


== false -> !match...()... for entire function.

> +			goto no_match;
> +		}
> +		break;
> +	case ustctl_atype_sequence:
> +		/* Match element type of the sequence. */
> +		if (match_ustctl_field_basic_type(&first->type.u.sequence.elem_type,
> +					&second->type.u.sequence.elem_type) == false) {
> +			goto no_match;
> +		}
> +
> +		/* Match length type of the sequence. */
> +		if (match_ustctl_field_basic_type(&first->type.u.sequence.length_type,
> +					&second->type.u.sequence.length_type) == false) {
> +			goto no_match;
> +		}
> +
> +		break;
> +	case ustctl_atype_array:
> +		/* Match element type of the array. */
> +		if (match_ustctl_field_basic_type(&first->type.u.array.elem_type,
> +					&second->type.u.array.elem_type) == false) {
> +			goto no_match;
> +		}
> +
> +		/* Match length of the array. */
> +		if (first->type.u.array.length != second->type.u.array.length) {
> +			goto no_match;
> +		}
> +
> +		break;
> +	case ustctl_atype_variant:
> +		/* Compare number of choice of the variants. */
> +		if (first->type.u.variant.nr_choices !=
> +					second->type.u.variant.nr_choices) {
> +			goto no_match;
> +		}
> +
> +		/* Compare tag name of the variants. */
> +		if (strncmp(first->type.u.variant.tag_name,
> +					second->type.u.variant.tag_name,
> +					LTTNG_UST_SYM_NAME_LEN)) {
> +			goto no_match;
> +		}
> +
> +		break;
> +	case ustctl_atype_struct:
> +		/* Compare number of fields of the structs. */
> +		if (first->type.u._struct.nr_fields != second->type.u._struct.nr_fields) {
> +			goto no_match;
> +		}
> +
> +		break;
> +	default:
> +		goto no_match;
> +	}
> +
> +	return true;
> +
> +no_match:
> +	return false;
> +}
> diff --git a/src/bin/lttng-sessiond/ust-field-utils.h
> b/src/bin/lttng-sessiond/ust-field-utils.h
> new file mode 100644
> index 0000000..13a1433
> --- /dev/null
> +++ b/src/bin/lttng-sessiond/ust-field-utils.h
> @@ -0,0 +1,29 @@
> +/*
> + * Copyright (C) 2018 - Francis Deslauriers francis.deslauriers at efficios.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License, version 2 only, as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program; if not, write to the Free Software Foundation, Inc., 51
> + * Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
> + */
> +
> +#ifndef LTTNG_UST_FIELD_UTILS_H
> +#define LTTNG_UST_FIELD_UTILS_H
> +
> +#include "ust-ctl.h"
> +
> +/*
> + * Compare two UST fields.
> + * Return 1 if both fields have identical definition, 0 otherwise.
> + */
> +int match_ustctl_field(struct ustctl_field *first, struct ustctl_field
> *second);
> +
> +#endif /* LTTNG_UST_FIELD_UTILS_H */
> diff --git a/src/bin/lttng-sessiond/ust-registry.c
> b/src/bin/lttng-sessiond/ust-registry.c
> index d63c750..e336b9e 100644
> --- a/src/bin/lttng-sessiond/ust-registry.c
> +++ b/src/bin/lttng-sessiond/ust-registry.c
> @@ -25,15 +25,18 @@
> 
> #include "ust-registry.h"
> #include "ust-app.h"
> +#include "ust-field-utils.h"
> #include "utils.h"
> #include "lttng-sessiond.h"
> #include "notification-thread-commands.h"
> 
> +
> /*
>  * Hash table match function for event in the registry.
>  */
> static int ht_match_event(struct cds_lfht_node *node, const void *_key)
> {
> +	int i;
> 	struct ust_registry_event *event;
> 	const struct ust_registry_event *key;

please try to put shorter declarations at the bottom, such as:

const struct ust_registry_event *key;
struct ust_registry_event *event;
int i;

Thanks,

Mathieu

> 
> @@ -44,15 +47,37 @@ static int ht_match_event(struct cds_lfht_node *node, const
> void *_key)
> 	assert(event);
> 	key = _key;
> 
> -	/* It has to be a perfect match. */
> +	/* It has to be a perfect match. First, compare the event names. */
> 	if (strncmp(event->name, key->name, sizeof(event->name))) {
> 		goto no_match;
> 	}
> 
> -	/* It has to be a perfect match. */
> -	if (strncmp(event->signature, key->signature,
> -			strlen(event->signature))) {
> +	/* Compare log levels. */
> +	if (event->loglevel_value != key->loglevel_value) {
> +		goto no_match;
> +	}
> +
> +	/* Compare the number of fields. */
> +	if (event->nr_fields != key->nr_fields) {
> +		goto no_match;
> +	}
> +
> +	/* Compare each field individually. */
> +	for (i = 0; i < event->nr_fields; i++) {
> +		if (match_ustctl_field(&event->fields[i], &key->fields[i]) == 0) {
> +			goto no_match;
> +		}
> +	}
> +
> +	/* Compare model URI. */
> +	if (event->model_emf_uri != NULL && key->model_emf_uri == NULL) {
> +		goto no_match;
> +	} else if(event->model_emf_uri == NULL && key->model_emf_uri != NULL) {
> 		goto no_match;
> +	} else if (event->model_emf_uri != NULL && key->model_emf_uri != NULL) {
> +		if (strcmp(event->model_emf_uri, key->model_emf_uri)) {
> +			goto no_match;
> +		}
> 	}
> 
> 	/* Match */
> @@ -64,15 +89,14 @@ no_match:
> 
> static unsigned long ht_hash_event(void *_key, unsigned long seed)
> {
> -	uint64_t xored_key;
> +	uint64_t hashed_key;
> 	struct ust_registry_event *key = _key;
> 
> 	assert(key);
> 
> -	xored_key = (uint64_t) (hash_key_str(key->name, seed) ^
> -			hash_key_str(key->signature, seed));
> +	hashed_key = (uint64_t) hash_key_str(key->name, seed);
> 
> -	return hash_key_u64(&xored_key, seed);
> +	return hash_key_u64(&hashed_key, seed);
> }
> 
> static int compare_enums(const struct ust_registry_enum *reg_enum_a,
> diff --git a/tests/unit/Makefile.am b/tests/unit/Makefile.am
> index 340dd93..2fb581b 100644
> --- a/tests/unit/Makefile.am
> +++ b/tests/unit/Makefile.am
> @@ -63,6 +63,7 @@
> UST_DATA_TRACE=$(top_builddir)/src/bin/lttng-sessiond/trace-ust.$(OBJEXT) \
> 	       $(top_builddir)/src/bin/lttng-sessiond/utils.$(OBJEXT) \
> 		   $(top_builddir)/src/bin/lttng-sessiond/buffer-registry.$(OBJEXT) \
> 		   $(top_builddir)/src/bin/lttng-sessiond/ust-registry.$(OBJEXT) \
> +		   $(top_builddir)/src/bin/lttng-sessiond/ust-field-utils.$(OBJEXT) \
> 		   $(top_builddir)/src/bin/lttng-sessiond/ust-metadata.$(OBJEXT) \
> 		   $(top_builddir)/src/bin/lttng-sessiond/ust-app.$(OBJEXT) \
> 		   $(top_builddir)/src/bin/lttng-sessiond/ust-consumer.$(OBJEXT) \
> --
> 2.7.4
> 
> _______________________________________________
> lttng-dev mailing list
> lttng-dev at lists.lttng.org
> https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
Francis Deslauriers Feb. 8, 2018, 4:16 p.m. UTC | #2
2018-02-07 15:43 GMT-05:00 Mathieu Desnoyers <mathieu.desnoyers at efficios.com>:
> ----- On Feb 7, 2018, at 2:36 PM, Francis Deslauriers francis.deslauriers at efficios.com wrote:
>
>> Events with different payloads but identical name and signatures could
>> lead to corrupted trace as the Session Daemon would consider them
>> identical and give them the same event ID.
>>
>> Events should be compared using the name, loglevel, fields and
>> model_emf_uri to ensure that their serialized layout is the same.
>
> model_emf_uri has no impact on the serialized layout, but does have an
> impact on what ends up in the metadata description for the event. The
> changelog should be adapated to clarify this.

I understand, how about this:
Fix: probes should be compared strictly by events metadata

Currently, events are compared using names and signatures. Events
with different payloads but identical name and signatures could
lead to corrupted trace as the Session Daemon would consider them
identical and give them the same event ID.

Events should be compared using the name, loglevel, fields and
model_emf_uri to ensure that their respective metadata is the same.

>
>>
>> Signed-off-by: Francis Deslauriers <francis.deslauriers at efficios.com>
>> ---
>> src/bin/lttng-sessiond/Makefile.am       |   3 +-
>> src/bin/lttng-sessiond/ust-field-utils.c | 261 +++++++++++++++++++++++++++++++
>> src/bin/lttng-sessiond/ust-field-utils.h |  29 ++++
>> src/bin/lttng-sessiond/ust-registry.c    |  40 ++++-
>> tests/unit/Makefile.am                   |   1 +
>> 5 files changed, 325 insertions(+), 9 deletions(-)
>> create mode 100644 src/bin/lttng-sessiond/ust-field-utils.c
>> create mode 100644 src/bin/lttng-sessiond/ust-field-utils.h
>>
>> diff --git a/src/bin/lttng-sessiond/Makefile.am
>> b/src/bin/lttng-sessiond/Makefile.am
>> index 413fe75..6fc1809 100644
>> --- a/src/bin/lttng-sessiond/Makefile.am
>> +++ b/src/bin/lttng-sessiond/Makefile.am
>> @@ -40,7 +40,8 @@ lttng_sessiond_SOURCES = utils.c utils.h \
>> if HAVE_LIBLTTNG_UST_CTL
>> lttng_sessiond_SOURCES += trace-ust.c ust-registry.c ust-app.c \
>>                       ust-consumer.c ust-consumer.h ust-thread.c \
>> -                     ust-metadata.c ust-clock.h agent-thread.c agent-thread.h
>> +                     ust-metadata.c ust-clock.h agent-thread.c agent-thread.h \
>> +                     ust-field-utils.h ust-field-utils.c
>> endif
>>
>> # Add main.c at the end for compile order
>> diff --git a/src/bin/lttng-sessiond/ust-field-utils.c
>> b/src/bin/lttng-sessiond/ust-field-utils.c
>> new file mode 100644
>> index 0000000..3c2da14
>> --- /dev/null
>> +++ b/src/bin/lttng-sessiond/ust-field-utils.c
>> @@ -0,0 +1,261 @@
>> +/*
>> + * Copyright (C) 2018 - Francis Deslauriers <francis.deslauriers at efficios.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms of the GNU General Public License, version 2 only, as
>> + * published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful, but WITHOUT
>> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
>> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
>> + * more details.
>> + *
>> + * You should have received a copy of the GNU General Public License along with
>> + * this program; if not, write to the Free Software Foundation, Inc., 51
>> + * Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
>> + */
>> +
>> +#include <string.h>
>> +#include <stdbool.h>
>> +
>> +#include "ust-field-utils.h"
>> +
>> +/*
>> + * The ustctl_field is made of a combinaison of C basic types
>
> combination
Done.
>
>> + * ustctl_basic_type and _ustctl_basic_type.
>> + *
>> + * ustctl_basic_type contains an enumeration describing the abstract type.
>> + * _ustctl_basic_type does _NOT_ contain an enumeration describing the
>> + * abstract type.
>> + *
>> + * A layer is needed to use the same code for both structures.
>> + * When dealing with _ustctl_basic_type, we need to use the abstract type of
>> + * the ustctl_type struct.
>> + */
>> +
>> +/*
>> + * Compare two ustctl_integer_type fields.
>> + * Returns 1 if both are identical.
>> + */
>> +static bool match_ustctl_field_integer(struct ustctl_integer_type *first,
>> +                     struct ustctl_integer_type *second)
>> +{
>> +     bool match = true;
>
> I don't like bitwise operators on C bool type. Is there anything that
> guarantees us that bool always uses the same bit to represent "true" ?
>
> Also, missing whiteline after the variable declaration.
Changed for simpler ifs.
>
>
>> +     match &= first->size == second->size;
>> +     match &= first->alignment == second->alignment;
>> +     match &= first->signedness == second->signedness;
>> +     match &= first->encoding == second->encoding;
>> +     match &= first->base == second->base;
>> +     match &= first->reverse_byte_order == second->reverse_byte_order;
>> +
>> +     return match;
>
>
> I'd prefer simply:
>
> if (first->size != second->size)
>    return false;
> .....
> return true;
>

Done.

>> +}
>> +
>> +/*
>> + * Compare two _ustctl_basic_type fields known to be of type integer.
>> + * Returns 1 if both are identical.
>> + */
>> +static bool match_ustctl_field_integer_from_raw_basic_type(
>> +                     union _ustctl_basic_type *first, union _ustctl_basic_type *second)
>> +{
>> +     return match_ustctl_field_integer(&first->integer, &second->integer);
>> +}
>> +
>> +/*
>> + * Compare two _ustctl_basic_type fields known to be of type enum.
>> + * Returns 1 if both are identical.
>> + */
>> +static bool match_ustctl_field_enum_from_raw_basic_type(
>> +                     union _ustctl_basic_type *first, union _ustctl_basic_type *second)
>> +{
>> +     /*
>> +      * Compare enumeration ID. Enumeration ID is provided to the application by
>> +      * the session daemon before event registration.
>> +      */
>> +     if (first->enumeration.id != second->enumeration.id) {
>> +             goto no_match;
>> +     }
>> +
>> +     /*
>> +      * Sanity check of the name and container type. Those were already checked
>> +      * during enum registration.
>> +      */
>> +     if (strncmp(first->enumeration.name, second->enumeration.name,
>> +                             LTTNG_UST_SYM_NAME_LEN)) {
>> +             goto no_match;
>> +     }
>> +     if (match_ustctl_field_integer(&first->enumeration.container_type,
>> +                             &second->enumeration.container_type) == false) {
>
> replace if (match_...() == false) {
>
> by
>
> if (!match...()) {
>

Done.

>> +             goto no_match;
>> +     }
>> +
>> +     return true;
>> +
>> +no_match:
>> +     return false;
>> +}
>> +
>> +/*
>> + * Compare two _ustctl_basic_type fields known to be of type string.
>> + * Returns 1 if both are identical.
>> + */
>> +static bool match_ustctl_field_string_from_raw_basic_type(
>> +                     union _ustctl_basic_type *first, union _ustctl_basic_type *second)
>> +{
>> +     return first->string.encoding == second->string.encoding;
>> +}
>> +
>> +/*
>> + * Compare two _ustctl_basic_type fields known to be of type float.
>> + * Returns 1 if both are identical.
>> + */
>> +static bool match_ustctl_field_float_from_raw_basic_type(
>> +                     union _ustctl_basic_type *first, union _ustctl_basic_type *second)
>> +{
>> +     bool match = true;
>
>
> same here about bitwise ops and whiteline.
Done.
>
>> +     match &= first->_float.exp_dig == second->_float.exp_dig;
>> +     match &= first->_float.mant_dig == second->_float.mant_dig;
>> +     match &= first->_float.reverse_byte_order ==
>> second->_float.reverse_byte_order;
>> +     match &= first->_float.alignment == second->_float.alignment;
>> +
>> +     return match;
>> +}
>> +
>> +/*
>> + * Compare two _ustctl_basic_type fields given their respective abstract types.
>> + * Returns 1 if both are identical.
>> + */
>> +static bool match_ustctl_field_raw_basic_type(
>> +                     enum ustctl_abstract_types first_atype,
>> +                     union _ustctl_basic_type *first,
>> +                     enum ustctl_abstract_types second_atype,
>> +                     union _ustctl_basic_type *second)
>> +{
>> +     if (first_atype != second_atype) {
>> +             goto no_match;
>> +     }
>> +
>> +     switch (first_atype) {
>> +     case ustctl_atype_integer:
>> +             if (match_ustctl_field_integer_from_raw_basic_type(first, second) == false) {
>> +                     goto no_match;
>> +             }
>> +             break;
>> +     case ustctl_atype_enum:
>> +             if (match_ustctl_field_enum_from_raw_basic_type(first, second) == false) {
>> +                     goto no_match;
>> +             }
>> +             break;
>> +     case ustctl_atype_string:
>> +             if (match_ustctl_field_string_from_raw_basic_type(first, second) == false) {
>> +                     goto no_match;
>> +             }
>> +             break;
>> +     case ustctl_atype_float:
>> +             if (match_ustctl_field_float_from_raw_basic_type(first, second) == false) {
>> +                     goto no_match;
>> +             }
>> +             break;
>> +     default:
>> +             goto no_match;
>> +     }
>> +
>> +     return true;
>> +
>> +no_match:
>> +     return false;
>> +}
>> +
>> +/*
>> + * Compatibility layer between the ustctl_basic_type struct and
>> + * _ustctl_basic_type union.
>> + */
>> +static bool match_ustctl_field_basic_type(struct ustctl_basic_type *first,
>> +                     struct ustctl_basic_type *second)
>> +{
>> +     return match_ustctl_field_raw_basic_type(first->atype, &first->u.basic,
>> +                             second->atype, &second->u.basic);
>> +}
>> +
>> +int match_ustctl_field(struct ustctl_field *first, struct ustctl_field *second)
>> +{
>> +     /* Check the name of the field is identical. */
>> +     if (strncmp(first->name, second->name, LTTNG_UST_SYM_NAME_LEN)) {
>> +             goto no_match;
>> +     }
>> +
>> +     /* Check the field type is identical. */
>> +     if (first->type.atype != second->type.atype) {
>> +             goto no_match;
>> +     }
>> +
>> +     /* Check the field layout. */
>> +     switch (first->type.atype) {
>> +     case ustctl_atype_integer:
>> +     case ustctl_atype_enum:
>> +     case ustctl_atype_string:
>> +     case ustctl_atype_float:
>> +             if (match_ustctl_field_raw_basic_type(first->type.atype,
>> +                                     &first->type.u.basic, second->type.atype,
>> +                                     &second->type.u.basic) == false) {
>
>
> == false -> !match...()... for entire function.
Done
>
>> +                     goto no_match;
>> +             }
>> +             break;
>> +     case ustctl_atype_sequence:
>> +             /* Match element type of the sequence. */
>> +             if (match_ustctl_field_basic_type(&first->type.u.sequence.elem_type,
>> +                                     &second->type.u.sequence.elem_type) == false) {
>> +                     goto no_match;
>> +             }
>> +
>> +             /* Match length type of the sequence. */
>> +             if (match_ustctl_field_basic_type(&first->type.u.sequence.length_type,
>> +                                     &second->type.u.sequence.length_type) == false) {
>> +                     goto no_match;
>> +             }
>> +
>> +             break;
>> +     case ustctl_atype_array:
>> +             /* Match element type of the array. */
>> +             if (match_ustctl_field_basic_type(&first->type.u.array.elem_type,
>> +                                     &second->type.u.array.elem_type) == false) {
>> +                     goto no_match;
>> +             }
>> +
>> +             /* Match length of the array. */
>> +             if (first->type.u.array.length != second->type.u.array.length) {
>> +                     goto no_match;
>> +             }
>> +
>> +             break;
>> +     case ustctl_atype_variant:
>> +             /* Compare number of choice of the variants. */
>> +             if (first->type.u.variant.nr_choices !=
>> +                                     second->type.u.variant.nr_choices) {
>> +                     goto no_match;
>> +             }
>> +
>> +             /* Compare tag name of the variants. */
>> +             if (strncmp(first->type.u.variant.tag_name,
>> +                                     second->type.u.variant.tag_name,
>> +                                     LTTNG_UST_SYM_NAME_LEN)) {
>> +                     goto no_match;
>> +             }
>> +
>> +             break;
>> +     case ustctl_atype_struct:
>> +             /* Compare number of fields of the structs. */
>> +             if (first->type.u._struct.nr_fields != second->type.u._struct.nr_fields) {
>> +                     goto no_match;
>> +             }
>> +
>> +             break;
>> +     default:
>> +             goto no_match;
>> +     }
>> +
>> +     return true;
>> +
>> +no_match:
>> +     return false;
>> +}
>> diff --git a/src/bin/lttng-sessiond/ust-field-utils.h
>> b/src/bin/lttng-sessiond/ust-field-utils.h
>> new file mode 100644
>> index 0000000..13a1433
>> --- /dev/null
>> +++ b/src/bin/lttng-sessiond/ust-field-utils.h
>> @@ -0,0 +1,29 @@
>> +/*
>> + * Copyright (C) 2018 - Francis Deslauriers francis.deslauriers at efficios.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms of the GNU General Public License, version 2 only, as
>> + * published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful, but WITHOUT
>> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
>> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
>> + * more details.
>> + *
>> + * You should have received a copy of the GNU General Public License along with
>> + * this program; if not, write to the Free Software Foundation, Inc., 51
>> + * Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
>> + */
>> +
>> +#ifndef LTTNG_UST_FIELD_UTILS_H
>> +#define LTTNG_UST_FIELD_UTILS_H
>> +
>> +#include "ust-ctl.h"
>> +
>> +/*
>> + * Compare two UST fields.
>> + * Return 1 if both fields have identical definition, 0 otherwise.
>> + */
>> +int match_ustctl_field(struct ustctl_field *first, struct ustctl_field
>> *second);
>> +
>> +#endif /* LTTNG_UST_FIELD_UTILS_H */
>> diff --git a/src/bin/lttng-sessiond/ust-registry.c
>> b/src/bin/lttng-sessiond/ust-registry.c
>> index d63c750..e336b9e 100644
>> --- a/src/bin/lttng-sessiond/ust-registry.c
>> +++ b/src/bin/lttng-sessiond/ust-registry.c
>> @@ -25,15 +25,18 @@
>>
>> #include "ust-registry.h"
>> #include "ust-app.h"
>> +#include "ust-field-utils.h"
>> #include "utils.h"
>> #include "lttng-sessiond.h"
>> #include "notification-thread-commands.h"
>>
>> +
>> /*
>>  * Hash table match function for event in the registry.
>>  */
>> static int ht_match_event(struct cds_lfht_node *node, const void *_key)
>> {
>> +     int i;
>>       struct ust_registry_event *event;
>>       const struct ust_registry_event *key;
>
> please try to put shorter declarations at the bottom, such as:
>
> const struct ust_registry_event *key;
> struct ust_registry_event *event;
> int i;
Done.
>
> Thanks,
>
> Mathieu
>
>>
>> @@ -44,15 +47,37 @@ static int ht_match_event(struct cds_lfht_node *node, const
>> void *_key)
>>       assert(event);
>>       key = _key;
>>
>> -     /* It has to be a perfect match. */
>> +     /* It has to be a perfect match. First, compare the event names. */
>>       if (strncmp(event->name, key->name, sizeof(event->name))) {
>>               goto no_match;
>>       }
>>
>> -     /* It has to be a perfect match. */
>> -     if (strncmp(event->signature, key->signature,
>> -                     strlen(event->signature))) {
>> +     /* Compare log levels. */
>> +     if (event->loglevel_value != key->loglevel_value) {
>> +             goto no_match;
>> +     }
>> +
>> +     /* Compare the number of fields. */
>> +     if (event->nr_fields != key->nr_fields) {
>> +             goto no_match;
>> +     }
>> +
>> +     /* Compare each field individually. */
>> +     for (i = 0; i < event->nr_fields; i++) {
>> +             if (match_ustctl_field(&event->fields[i], &key->fields[i]) == 0) {
>> +                     goto no_match;
>> +             }
>> +     }
>> +
>> +     /* Compare model URI. */
>> +     if (event->model_emf_uri != NULL && key->model_emf_uri == NULL) {
>> +             goto no_match;
>> +     } else if(event->model_emf_uri == NULL && key->model_emf_uri != NULL) {
>>               goto no_match;
>> +     } else if (event->model_emf_uri != NULL && key->model_emf_uri != NULL) {
>> +             if (strcmp(event->model_emf_uri, key->model_emf_uri)) {
>> +                     goto no_match;
>> +             }
>>       }
>>
>>       /* Match */
>> @@ -64,15 +89,14 @@ no_match:
>>
>> static unsigned long ht_hash_event(void *_key, unsigned long seed)
>> {
>> -     uint64_t xored_key;
>> +     uint64_t hashed_key;
>>       struct ust_registry_event *key = _key;
>>
>>       assert(key);
>>
>> -     xored_key = (uint64_t) (hash_key_str(key->name, seed) ^
>> -                     hash_key_str(key->signature, seed));
>> +     hashed_key = (uint64_t) hash_key_str(key->name, seed);
>>
>> -     return hash_key_u64(&xored_key, seed);
>> +     return hash_key_u64(&hashed_key, seed);
>> }
>>
>> static int compare_enums(const struct ust_registry_enum *reg_enum_a,
>> diff --git a/tests/unit/Makefile.am b/tests/unit/Makefile.am
>> index 340dd93..2fb581b 100644
>> --- a/tests/unit/Makefile.am
>> +++ b/tests/unit/Makefile.am
>> @@ -63,6 +63,7 @@
>> UST_DATA_TRACE=$(top_builddir)/src/bin/lttng-sessiond/trace-ust.$(OBJEXT) \
>>              $(top_builddir)/src/bin/lttng-sessiond/utils.$(OBJEXT) \
>>                  $(top_builddir)/src/bin/lttng-sessiond/buffer-registry.$(OBJEXT) \
>>                  $(top_builddir)/src/bin/lttng-sessiond/ust-registry.$(OBJEXT) \
>> +                $(top_builddir)/src/bin/lttng-sessiond/ust-field-utils.$(OBJEXT) \
>>                  $(top_builddir)/src/bin/lttng-sessiond/ust-metadata.$(OBJEXT) \
>>                  $(top_builddir)/src/bin/lttng-sessiond/ust-app.$(OBJEXT) \
>>                  $(top_builddir)/src/bin/lttng-sessiond/ust-consumer.$(OBJEXT) \
>> --
>> 2.7.4
>>
>> _______________________________________________
>> lttng-dev mailing list
>> lttng-dev at lists.lttng.org
>> https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
>
> --
> Mathieu Desnoyers
> EfficiOS Inc.
> http://www.efficios.com
diff mbox series

Patch

diff --git a/src/bin/lttng-sessiond/Makefile.am b/src/bin/lttng-sessiond/Makefile.am
index 413fe75..6fc1809 100644
--- a/src/bin/lttng-sessiond/Makefile.am
+++ b/src/bin/lttng-sessiond/Makefile.am
@@ -40,7 +40,8 @@  lttng_sessiond_SOURCES = utils.c utils.h \
 if HAVE_LIBLTTNG_UST_CTL
 lttng_sessiond_SOURCES += trace-ust.c ust-registry.c ust-app.c \
 			ust-consumer.c ust-consumer.h ust-thread.c \
-			ust-metadata.c ust-clock.h agent-thread.c agent-thread.h
+			ust-metadata.c ust-clock.h agent-thread.c agent-thread.h \
+			ust-field-utils.h ust-field-utils.c
 endif
 
 # Add main.c at the end for compile order
diff --git a/src/bin/lttng-sessiond/ust-field-utils.c b/src/bin/lttng-sessiond/ust-field-utils.c
new file mode 100644
index 0000000..3c2da14
--- /dev/null
+++ b/src/bin/lttng-sessiond/ust-field-utils.c
@@ -0,0 +1,261 @@ 
+/*
+ * Copyright (C) 2018 - Francis Deslauriers <francis.deslauriers at efficios.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License, version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program; if not, write to the Free Software Foundation, Inc., 51
+ * Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#include <string.h>
+#include <stdbool.h>
+
+#include "ust-field-utils.h"
+
+/*
+ * The ustctl_field is made of a combinaison of C basic types
+ * ustctl_basic_type and _ustctl_basic_type.
+ *
+ * ustctl_basic_type contains an enumeration describing the abstract type.
+ * _ustctl_basic_type does _NOT_ contain an enumeration describing the
+ * abstract type.
+ *
+ * A layer is needed to use the same code for both structures.
+ * When dealing with _ustctl_basic_type, we need to use the abstract type of
+ * the ustctl_type struct.
+ */
+
+/*
+ * Compare two ustctl_integer_type fields.
+ * Returns 1 if both are identical.
+ */
+static bool match_ustctl_field_integer(struct ustctl_integer_type *first,
+			struct ustctl_integer_type *second)
+{
+	bool match = true;
+	match &= first->size == second->size;
+	match &= first->alignment == second->alignment;
+	match &= first->signedness == second->signedness;
+	match &= first->encoding == second->encoding;
+	match &= first->base == second->base;
+	match &= first->reverse_byte_order == second->reverse_byte_order;
+
+	return match;
+}
+
+/*
+ * Compare two _ustctl_basic_type fields known to be of type integer.
+ * Returns 1 if both are identical.
+ */
+static bool match_ustctl_field_integer_from_raw_basic_type(
+			union _ustctl_basic_type *first, union _ustctl_basic_type *second)
+{
+	return match_ustctl_field_integer(&first->integer, &second->integer);
+}
+
+/*
+ * Compare two _ustctl_basic_type fields known to be of type enum.
+ * Returns 1 if both are identical.
+ */
+static bool match_ustctl_field_enum_from_raw_basic_type(
+			union _ustctl_basic_type *first, union _ustctl_basic_type *second)
+{
+	/*
+	 * Compare enumeration ID. Enumeration ID is provided to the application by
+	 * the session daemon before event registration.
+	 */
+	if (first->enumeration.id != second->enumeration.id) {
+		goto no_match;
+	}
+
+	/*
+	 * Sanity check of the name and container type. Those were already checked
+	 * during enum registration.
+	 */
+	if (strncmp(first->enumeration.name, second->enumeration.name,
+				LTTNG_UST_SYM_NAME_LEN)) {
+		goto no_match;
+	}
+	if (match_ustctl_field_integer(&first->enumeration.container_type,
+				&second->enumeration.container_type) == false) {
+		goto no_match;
+	}
+
+	return true;
+
+no_match:
+	return false;
+}
+
+/*
+ * Compare two _ustctl_basic_type fields known to be of type string.
+ * Returns 1 if both are identical.
+ */
+static bool match_ustctl_field_string_from_raw_basic_type(
+			union _ustctl_basic_type *first, union _ustctl_basic_type *second)
+{
+	return first->string.encoding == second->string.encoding;
+}
+
+/*
+ * Compare two _ustctl_basic_type fields known to be of type float.
+ * Returns 1 if both are identical.
+ */
+static bool match_ustctl_field_float_from_raw_basic_type(
+			union _ustctl_basic_type *first, union _ustctl_basic_type *second)
+{
+	bool match = true;
+	match &= first->_float.exp_dig == second->_float.exp_dig;
+	match &= first->_float.mant_dig == second->_float.mant_dig;
+	match &= first->_float.reverse_byte_order == second->_float.reverse_byte_order;
+	match &= first->_float.alignment == second->_float.alignment;
+
+	return match;
+}
+
+/*
+ * Compare two _ustctl_basic_type fields given their respective abstract types.
+ * Returns 1 if both are identical.
+ */
+static bool match_ustctl_field_raw_basic_type(
+			enum ustctl_abstract_types first_atype,
+			union _ustctl_basic_type *first,
+			enum ustctl_abstract_types second_atype,
+			union _ustctl_basic_type *second)
+{
+	if (first_atype != second_atype) {
+		goto no_match;
+	}
+
+	switch (first_atype) {
+	case ustctl_atype_integer:
+		if (match_ustctl_field_integer_from_raw_basic_type(first, second) == false) {
+			goto no_match;
+		}
+		break;
+	case ustctl_atype_enum:
+		if (match_ustctl_field_enum_from_raw_basic_type(first, second) == false) {
+			goto no_match;
+		}
+		break;
+	case ustctl_atype_string:
+		if (match_ustctl_field_string_from_raw_basic_type(first, second) == false) {
+			goto no_match;
+		}
+		break;
+	case ustctl_atype_float:
+		if (match_ustctl_field_float_from_raw_basic_type(first, second) == false) {
+			goto no_match;
+		}
+		break;
+	default:
+		goto no_match;
+	}
+
+	return true;
+
+no_match:
+	return false;
+}
+
+/*
+ * Compatibility layer between the ustctl_basic_type struct and
+ * _ustctl_basic_type union.
+ */
+static bool match_ustctl_field_basic_type(struct ustctl_basic_type *first,
+			struct ustctl_basic_type *second)
+{
+	return match_ustctl_field_raw_basic_type(first->atype, &first->u.basic,
+				second->atype, &second->u.basic);
+}
+
+int match_ustctl_field(struct ustctl_field *first, struct ustctl_field *second)
+{
+	/* Check the name of the field is identical. */
+	if (strncmp(first->name, second->name, LTTNG_UST_SYM_NAME_LEN)) {
+		goto no_match;
+	}
+
+	/* Check the field type is identical. */
+	if (first->type.atype != second->type.atype) {
+		goto no_match;
+	}
+
+	/* Check the field layout. */
+	switch (first->type.atype) {
+	case ustctl_atype_integer:
+	case ustctl_atype_enum:
+	case ustctl_atype_string:
+	case ustctl_atype_float:
+		if (match_ustctl_field_raw_basic_type(first->type.atype,
+					&first->type.u.basic, second->type.atype,
+					&second->type.u.basic) == false) {
+			goto no_match;
+		}
+		break;
+	case ustctl_atype_sequence:
+		/* Match element type of the sequence. */
+		if (match_ustctl_field_basic_type(&first->type.u.sequence.elem_type,
+					&second->type.u.sequence.elem_type) == false) {
+			goto no_match;
+		}
+
+		/* Match length type of the sequence. */
+		if (match_ustctl_field_basic_type(&first->type.u.sequence.length_type,
+					&second->type.u.sequence.length_type) == false) {
+			goto no_match;
+		}
+
+		break;
+	case ustctl_atype_array:
+		/* Match element type of the array. */
+		if (match_ustctl_field_basic_type(&first->type.u.array.elem_type,
+					&second->type.u.array.elem_type) == false) {
+			goto no_match;
+		}
+
+		/* Match length of the array. */
+		if (first->type.u.array.length != second->type.u.array.length) {
+			goto no_match;
+		}
+
+		break;
+	case ustctl_atype_variant:
+		/* Compare number of choice of the variants. */
+		if (first->type.u.variant.nr_choices !=
+					second->type.u.variant.nr_choices) {
+			goto no_match;
+		}
+
+		/* Compare tag name of the variants. */
+		if (strncmp(first->type.u.variant.tag_name,
+					second->type.u.variant.tag_name,
+					LTTNG_UST_SYM_NAME_LEN)) {
+			goto no_match;
+		}
+
+		break;
+	case ustctl_atype_struct:
+		/* Compare number of fields of the structs. */
+		if (first->type.u._struct.nr_fields != second->type.u._struct.nr_fields) {
+			goto no_match;
+		}
+
+		break;
+	default:
+		goto no_match;
+	}
+
+	return true;
+
+no_match:
+	return false;
+}
diff --git a/src/bin/lttng-sessiond/ust-field-utils.h b/src/bin/lttng-sessiond/ust-field-utils.h
new file mode 100644
index 0000000..13a1433
--- /dev/null
+++ b/src/bin/lttng-sessiond/ust-field-utils.h
@@ -0,0 +1,29 @@ 
+/*
+ * Copyright (C) 2018 - Francis Deslauriers francis.deslauriers at efficios.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License, version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program; if not, write to the Free Software Foundation, Inc., 51
+ * Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#ifndef LTTNG_UST_FIELD_UTILS_H
+#define LTTNG_UST_FIELD_UTILS_H
+
+#include "ust-ctl.h"
+
+/*
+ * Compare two UST fields.
+ * Return 1 if both fields have identical definition, 0 otherwise.
+ */
+int match_ustctl_field(struct ustctl_field *first, struct ustctl_field *second);
+
+#endif /* LTTNG_UST_FIELD_UTILS_H */
diff --git a/src/bin/lttng-sessiond/ust-registry.c b/src/bin/lttng-sessiond/ust-registry.c
index d63c750..e336b9e 100644
--- a/src/bin/lttng-sessiond/ust-registry.c
+++ b/src/bin/lttng-sessiond/ust-registry.c
@@ -25,15 +25,18 @@ 
 
 #include "ust-registry.h"
 #include "ust-app.h"
+#include "ust-field-utils.h"
 #include "utils.h"
 #include "lttng-sessiond.h"
 #include "notification-thread-commands.h"
 
+
 /*
  * Hash table match function for event in the registry.
  */
 static int ht_match_event(struct cds_lfht_node *node, const void *_key)
 {
+	int i;
 	struct ust_registry_event *event;
 	const struct ust_registry_event *key;
 
@@ -44,15 +47,37 @@  static int ht_match_event(struct cds_lfht_node *node, const void *_key)
 	assert(event);
 	key = _key;
 
-	/* It has to be a perfect match. */
+	/* It has to be a perfect match. First, compare the event names. */
 	if (strncmp(event->name, key->name, sizeof(event->name))) {
 		goto no_match;
 	}
 
-	/* It has to be a perfect match. */
-	if (strncmp(event->signature, key->signature,
-			strlen(event->signature))) {
+	/* Compare log levels. */
+	if (event->loglevel_value != key->loglevel_value) {
+		goto no_match;
+	}
+
+	/* Compare the number of fields. */
+	if (event->nr_fields != key->nr_fields) {
+		goto no_match;
+	}
+
+	/* Compare each field individually. */
+	for (i = 0; i < event->nr_fields; i++) {
+		if (match_ustctl_field(&event->fields[i], &key->fields[i]) == 0) {
+			goto no_match;
+		}
+	}
+
+	/* Compare model URI. */
+	if (event->model_emf_uri != NULL && key->model_emf_uri == NULL) {
+		goto no_match;
+	} else if(event->model_emf_uri == NULL && key->model_emf_uri != NULL) {
 		goto no_match;
+	} else if (event->model_emf_uri != NULL && key->model_emf_uri != NULL) {
+		if (strcmp(event->model_emf_uri, key->model_emf_uri)) {
+			goto no_match;
+		}
 	}
 
 	/* Match */
@@ -64,15 +89,14 @@  no_match:
 
 static unsigned long ht_hash_event(void *_key, unsigned long seed)
 {
-	uint64_t xored_key;
+	uint64_t hashed_key;
 	struct ust_registry_event *key = _key;
 
 	assert(key);
 
-	xored_key = (uint64_t) (hash_key_str(key->name, seed) ^
-			hash_key_str(key->signature, seed));
+	hashed_key = (uint64_t) hash_key_str(key->name, seed);
 
-	return hash_key_u64(&xored_key, seed);
+	return hash_key_u64(&hashed_key, seed);
 }
 
 static int compare_enums(const struct ust_registry_enum *reg_enum_a,
diff --git a/tests/unit/Makefile.am b/tests/unit/Makefile.am
index 340dd93..2fb581b 100644
--- a/tests/unit/Makefile.am
+++ b/tests/unit/Makefile.am
@@ -63,6 +63,7 @@  UST_DATA_TRACE=$(top_builddir)/src/bin/lttng-sessiond/trace-ust.$(OBJEXT) \
 	       $(top_builddir)/src/bin/lttng-sessiond/utils.$(OBJEXT) \
 		   $(top_builddir)/src/bin/lttng-sessiond/buffer-registry.$(OBJEXT) \
 		   $(top_builddir)/src/bin/lttng-sessiond/ust-registry.$(OBJEXT) \
+		   $(top_builddir)/src/bin/lttng-sessiond/ust-field-utils.$(OBJEXT) \
 		   $(top_builddir)/src/bin/lttng-sessiond/ust-metadata.$(OBJEXT) \
 		   $(top_builddir)/src/bin/lttng-sessiond/ust-app.$(OBJEXT) \
 		   $(top_builddir)/src/bin/lttng-sessiond/ust-consumer.$(OBJEXT) \