diff mbox series

[lttng-tools,2/5] Fix: should pass the reg_enum_lookup pointer directly

Message ID 1518032219-28072-3-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
As the ht_hash_enum and ht_match_enum functions are not changing the
pointer there is no need to pass the address of the pointer.

Signed-off-by: Francis Deslauriers <francis.deslauriers at efficios.com>
---
 src/bin/lttng-sessiond/ust-registry.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

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

> As the ht_hash_enum and ht_match_enum functions are not changing the
> pointer there is no need to pass the address of the pointer.

The changelog seems too nice. The current situation is that the arguments
passed to the cds_lfht_lookup() function do not match the args expected by ht_has_enum
and ht_match_enum, and by chance we probably always end up comparing with garbage ?

The changelog should better describe the current problem.

Thanks,

Mathieu

> 
> Signed-off-by: Francis Deslauriers <francis.deslauriers at efficios.com>
> ---
> src/bin/lttng-sessiond/ust-registry.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/bin/lttng-sessiond/ust-registry.c
> b/src/bin/lttng-sessiond/ust-registry.c
> index e336b9e..cd6fdac 100644
> --- a/src/bin/lttng-sessiond/ust-registry.c
> +++ b/src/bin/lttng-sessiond/ust-registry.c
> @@ -558,8 +558,8 @@ struct ust_registry_enum *
> 	struct lttng_ht_iter iter;
> 
> 	cds_lfht_lookup(session->enums->ht,
> -			ht_hash_enum((void *) &reg_enum_lookup, lttng_ht_seed),
> -			ht_match_enum, &reg_enum_lookup, &iter.iter);
> +			ht_hash_enum((void *) reg_enum_lookup, lttng_ht_seed),
> +			ht_match_enum, reg_enum_lookup, &iter.iter);
> 	node = lttng_ht_iter_get_node_str(&iter);
> 	if (!node) {
> 	        goto end;
> --
> 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:42 p.m. UTC | #2
2018-02-07 15:48 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:
>
>> As the ht_hash_enum and ht_match_enum functions are not changing the
>> pointer there is no need to pass the address of the pointer.
>
> The changelog seems too nice. The current situation is that the arguments
> passed to the cds_lfht_lookup() function do not match the args expected by ht_has_enum
> and ht_match_enum, and by chance we probably always end up comparing with garbage ?
>
> The changelog should better describe the current problem.

Here is a revised version:

Fix: calling ht_{hash, match}_enum with wrong argument

ht_hash_enum and ht_match_enum are currently called with the address of the
pointer to a ust_registry_enum rather than the expected pointer to a
ust_registry_enum. This means that those function calls would end up
using garbage for hashing and comparing.


Thank you,
Francis

>
> Thanks,
>
> Mathieu
>
>>
>> Signed-off-by: Francis Deslauriers <francis.deslauriers at efficios.com>
>> ---
>> src/bin/lttng-sessiond/ust-registry.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/bin/lttng-sessiond/ust-registry.c
>> b/src/bin/lttng-sessiond/ust-registry.c
>> index e336b9e..cd6fdac 100644
>> --- a/src/bin/lttng-sessiond/ust-registry.c
>> +++ b/src/bin/lttng-sessiond/ust-registry.c
>> @@ -558,8 +558,8 @@ struct ust_registry_enum *
>>       struct lttng_ht_iter iter;
>>
>>       cds_lfht_lookup(session->enums->ht,
>> -                     ht_hash_enum((void *) &reg_enum_lookup, lttng_ht_seed),
>> -                     ht_match_enum, &reg_enum_lookup, &iter.iter);
>> +                     ht_hash_enum((void *) reg_enum_lookup, lttng_ht_seed),
>> +                     ht_match_enum, reg_enum_lookup, &iter.iter);
>>       node = lttng_ht_iter_get_node_str(&iter);
>>       if (!node) {
>>               goto end;
>> --
>> 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/ust-registry.c b/src/bin/lttng-sessiond/ust-registry.c
index e336b9e..cd6fdac 100644
--- a/src/bin/lttng-sessiond/ust-registry.c
+++ b/src/bin/lttng-sessiond/ust-registry.c
@@ -558,8 +558,8 @@  struct ust_registry_enum *
 	struct lttng_ht_iter iter;
 
 	cds_lfht_lookup(session->enums->ht,
-			ht_hash_enum((void *) &reg_enum_lookup, lttng_ht_seed),
-			ht_match_enum, &reg_enum_lookup, &iter.iter);
+			ht_hash_enum((void *) reg_enum_lookup, lttng_ht_seed),
+			ht_match_enum, reg_enum_lookup, &iter.iter);
 	node = lttng_ht_iter_get_node_str(&iter);
 	if (!node) {
 	        goto end;