Re: [PATCH 7/7] perf data: Fix signess of value

[Arnaldo Carvalho de Melo <acme@kernel.org>]

Em Sat, Apr 18, 2015 at 05:50:20PM +0200, Jiri Olsa escreveu:
> From: Wang Nan <wangnan0@huawei.com>
> 
> When converting int values, perf first extractes it to a ulonglong, then
> feeds it to babeltrace as a signed value. For negative 32 bit values
> (for example, return values of failed syscalls), the extracted data
> should be something like 0xfffffffe (-2). It becomes a large int64
> value. Babeltrace denies to insert it with
> bt_ctf_field_signed_integer_set_value() because it is larger than
> 0x7fffffff, the largest positive value a signed 32 bit int can be.

There is no such word "signess", it is "signedness", fixing this up.
Humm, it seems there is such a word indeed:

http://www.urbandictionary.com/define.php?term=Signess

But I bet this is the one we want:

http://en.wikipedia.org/wiki/Signedness

Right? :-)

- Arnaldo
 
> This patch introduces adjust_signess(), which fills high bits of
> ulonglong with 1 if the value is negative.
> 
> Signed-off-by: Wang Nan <wangnan0@huawei.com>
> Link: http://lkml.kernel.org/n/tip-bq2fguw8pm8s5rr741mmgias@git.kernel.org
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  tools/perf/util/data-convert-bt.c | 64 +++++++++++++++++++++++++++++++--------
>  1 file changed, 52 insertions(+), 12 deletions(-)
> 
> diff --git a/tools/perf/util/data-convert-bt.c b/tools/perf/util/data-convert-bt.c
> index 8eda4ed628e7..977cc3f98d8f 100644
> --- a/tools/perf/util/data-convert-bt.c
> +++ b/tools/perf/util/data-convert-bt.c
> @@ -166,6 +166,43 @@ get_tracepoint_field_type(struct ctf_writer *cw, struct format_field *field)
>  		return cw->data.u32;
>  }
>  
> +static unsigned long long adjust_signess(unsigned long long value_int, int size)
> +{
> +	unsigned long long value_mask;
> +
> +	/*
> +	 * value_mask = (1 << (size * 8 - 1)) - 1.
> +	 * Directly set value_mask for code readers.
> +	 */
> +	switch (size) {
> +	case 1:
> +		value_mask = 0x7fULL;
> +		break;
> +	case 2:
> +		value_mask = 0x7fffULL;
> +		break;
> +	case 4:
> +		value_mask = 0x7fffffffULL;
> +		break;
> +	case 8:
> +		/*
> +		 * For 64 bit value, return it self. There is no need
> +		 * to fill high bit.
> +		 */
> +		/* Fall through */
> +	default:
> +		/* BUG! */
> +		return value_int;
> +	}
> +
> +	/* If it is a positive value, don't adjust. */
> +	if ((value_int & (~0ULL - value_mask)) == 0)
> +		return value_int;
> +
> +	/* Fill upper part of value_int with 1 to make it a negative long long. */
> +	return (value_int & value_mask) | ~value_mask;
> +}
> +
>  static int add_tracepoint_field_value(struct ctf_writer *cw,
>  				      struct bt_ctf_event_class *event_class,
>  				      struct bt_ctf_event *event,
> @@ -177,7 +214,6 @@ static int add_tracepoint_field_value(struct ctf_writer *cw,
>  	struct bt_ctf_field *field;
>  	const char *name = fmtf->name;
>  	void *data = sample->raw_data;
> -	unsigned long long value_int;
>  	unsigned long flags = fmtf->flags;
>  	unsigned int n_items;
>  	unsigned int i;
> @@ -222,11 +258,6 @@ static int add_tracepoint_field_value(struct ctf_writer *cw,
>  	type = get_tracepoint_field_type(cw, fmtf);
>  
>  	for (i = 0; i < n_items; i++) {
> -		if (!(flags & FIELD_IS_STRING))
> -			value_int = pevent_read_number(
> -					fmtf->event->pevent,
> -					data + offset + i * len, len);
> -
>  		if (flags & FIELD_IS_ARRAY)
>  			field = bt_ctf_field_array_get_field(array_field, i);
>  		else
> @@ -240,12 +271,21 @@ static int add_tracepoint_field_value(struct ctf_writer *cw,
>  		if (flags & FIELD_IS_STRING)
>  			ret = bt_ctf_field_string_set_value(field,
>  					data + offset + i * len);
> -		else if (!(flags & FIELD_IS_SIGNED))
> -			ret = bt_ctf_field_unsigned_integer_set_value(
> -					field, value_int);
> -		else
> -			ret = bt_ctf_field_signed_integer_set_value(
> -					field, value_int);
> +		else {
> +			unsigned long long value_int;
> +
> +			value_int = pevent_read_number(
> +					fmtf->event->pevent,
> +					data + offset + i * len, len);
> +
> +			if (!(flags & FIELD_IS_SIGNED))
> +				ret = bt_ctf_field_unsigned_integer_set_value(
> +						field, value_int);
> +			else
> +				ret = bt_ctf_field_signed_integer_set_value(
> +						field, adjust_signess(value_int, len));
> +		}
> +
>  		if (ret) {
>  			pr_err("failed to set file value %s\n", name);
>  			goto err_put_field;
> -- 
> 1.9.3