Re: [PATCH v10 2/4] trace/objtrace: Get the value of the object

[Masami Hiramatsu (Google) <mhiramat@kernel.org>]

Hi Jeff,

On Fri, 13 May 2022 01:00:06 +0800
Jeff Xie <xiehuan09@gmail.com> wrote:

[...]
> @@ -175,9 +271,27 @@ trace_object_trigger(struct event_trigger_data *data,
>  
>  	field = obj_data->field;
>  	memcpy(&obj, rec + field->offset, sizeof(obj));
> -	set_trace_object(obj, tr);
> +	/* set the offset from the special object and the type size of the value*/
> +	set_trace_object(obj, obj_data->obj_offset,
> +			obj_data->obj_value_type_size, tr);
>  }
>  
> +static const struct objtrace_fetch_type objtrace_fetch_types[] = {
> +	{"u8", 1},
> +	{"s8", 1},
> +	{"x8", 1},
> +	{"u16", 2},
> +	{"s16", 2},
> +	{"x16", 2},
> +	{"u32", 4},
> +	{"s32", 4},
> +	{"x32", 4},
> +	{"u64", 8},
> +	{"s64", 8},
> +	{"x64", 8},

Hmm, as far as I can see, you don't distinguish the prefix 'u','s','x'.
If so, please support only 'x' at this moment. kprobe events supports
those types, and it distinguishes the types when printing the logged
data. E.g. 's16' shows '-1' for 0xffff, but 'x16' shows '0xffff'.
You can add another patch to support such different types afterwards.

> +	{}

If this array is null terminated, please explictly do that, like

	{NULL, 0},

for readability.

Thank you,

-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>