Re: [PATCH 10/10] perf script python: Fix string vs byte array resolving

[Steven Rostedt]

On Sun, 10 Jul 2016 13:08:02 +0200
Jiri Olsa <jolsa@xxxxxxxxxx> wrote:

> Jirka reported that python code returns all arrays as strings.
> This makes impossible to get all items for byte array tracepoint
> field containing 0x00 value item.
> 
> Fixing this by scanning full length of the array and returning
> it as PyByteArray object in case non printable byte is found.
> 
> Cc: Steven Rostedt (Red Hat) <rostedt@xxxxxxxxxxx>
> Reported-and-tested-by: Jiri Pirko <jiri@xxxxxxxxxxxx>
> Link: http://lkml.kernel.org/n/tip-22f4vhhz5uytegkggy1on8u3@xxxxxxxxxxxxxx
> Signed-off-by: Jiri Olsa <jolsa@xxxxxxxxxx>
> ---
>  .../util/scripting-engines/trace-event-python.c    | 34 ++++++++++++++++++----
>  1 file changed, 28 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/perf/util/scripting-engines/trace-event-python.c b/tools/perf/util/scripting-engines/trace-event-python.c
> index ff134700bf30..75e9790ebb96 100644
> --- a/tools/perf/util/scripting-engines/trace-event-python.c
> +++ b/tools/perf/util/scripting-engines/trace-event-python.c
> @@ -386,6 +386,16 @@ exit:
>  	return pylist;
>  }
>  
> +static int is_printable_array(char *p, unsigned int len)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < len; i++)
> +		if (!isprint(p[i]) && !isspace(p[i]))
> +			return 0;

I believe this will return the wrong result if you pass in a string
that has '\0' in it. Try it out:

	char a[] = "hello";

	printf("a=%s is %d\n", a, is_printable_array(a, sizeof(a)));

and see what you get?

-- Steve

> +
> +	return 1;
> +}
>  
>  static void python_process_tracepoint(struct perf_sample *sample,
>  				      struct perf_evsel *evsel,
> @@ -457,14 +467,26 @@ static void python_process_tracepoint(struct perf_sample *sample,
>  		pydict_set_item_string_decref(dict, "common_callchain", callchain);
>  	}
>  	for (field = event->format.fields; field; field = field->next) {
> -		if (field->flags & FIELD_IS_STRING) {
> -			int offset;
> +		unsigned int offset, len;
> +		unsigned long long val;
> +
> +		if (field->flags & FIELD_IS_ARRAY) {
> +			offset = field->offset;
> +			len    = field->size;
>  			if (field->flags & FIELD_IS_DYNAMIC) {
> -				offset = *(int *)(data + field->offset);
> +				val     = pevent_read_number(scripting_context->pevent,
> +							     data + offset, len);
> +				offset  = val;
> +				len     = offset >> 16;
>  				offset &= 0xffff;
> -			} else
> -				offset = field->offset;
> -			obj = PyString_FromString((char *)data + offset);
> +			}
> +			if (field->flags & FIELD_IS_STRING &&
> +			    is_printable_array(data + offset, len)) {
> +				obj = PyString_FromString((char *) data + offset);
> +			} else {
> +				obj = PyByteArray_FromStringAndSize((const char *) data + offset, len);
> +				field->flags &= ~FIELD_IS_STRING;
> +			}
>  		} else { /* FIELD_IS_NUMERIC */
>  			obj = get_field_numeric_entry(event, field, data);
>  		}