public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Jiri Olsa <jolsa@redhat.com>
To: Philippe Proulx <eeppeliteloop@gmail.com>
Cc: linux-kernel@vger.kernel.org, "Wang Nan" <wangnan0@huawei.com>,
	"Jérémie Galarneau" <jeremie.galarneau@gmail.com>,
	"David S . Miller" <davem@davemloft.net>,
	"Alexei Starovoitov" <ast@kernel.org>,
	"Brendan Gregg" <brendan.d.gregg@gmail.com>,
	"Jiri Olsa" <jolsa@kernel.org>,
	"Masami Hiramatsu" <masami.hiramatsu.pt@hitachi.com>,
	"Namhyung Kim" <namhyung@kernel.org>,
	"Zefan Li" <lizefan@huawei.com>,
	pi3orama@163.com
Subject: Re: [PATCH] perf data convert: fix possible leak
Date: Mon, 24 Oct 2016 23:08:41 +0200	[thread overview]
Message-ID: <20161024210841.GA2382@krava> (raw)
In-Reply-To: <20161024192306.18566-1-eeppeliteloop@gmail.com>

On Mon, Oct 24, 2016 at 03:23:06PM -0400, Philippe Proulx wrote:
> This patch removes the need to manually update the static size of
> data.array in struct ctf_writer.
> 
> Signed-off-by: Philippe Proulx <eeppeliteloop@gmail.com>

Acked-by: Jiri Olsa <jolsa@kernel.org>

thanks,
jirka

> ---
>  tools/perf/util/data-convert-bt.c | 65 ++++++++++++++++++++-------------------
>  1 file changed, 33 insertions(+), 32 deletions(-)
> 
> diff --git a/tools/perf/util/data-convert-bt.c b/tools/perf/util/data-convert-bt.c
> index 7123f4d..6c04ad6 100644
> --- a/tools/perf/util/data-convert-bt.c
> +++ b/tools/perf/util/data-convert-bt.c
> @@ -58,7 +58,7 @@ struct ctf_writer {
>  
>  	/* data types */
>  	union {
> -		struct {
> +		struct field_types {
>  			struct bt_ctf_field_type	*s64;
>  			struct bt_ctf_field_type	*u64;
>  			struct bt_ctf_field_type	*s32;
> @@ -66,8 +66,9 @@ struct ctf_writer {
>  			struct bt_ctf_field_type	*string;
>  			struct bt_ctf_field_type	*u32_hex;
>  			struct bt_ctf_field_type	*u64_hex;
> -		};
> -		struct bt_ctf_field_type *array[6];
> +		} fts;
> +		struct bt_ctf_field_type *array[sizeof(struct field_types) /
> +					sizeof(struct bt_ctf_field_type *)];
>  	} data;
>  	struct bt_ctf_event_class	*comm_class;
>  	struct bt_ctf_event_class	*exit_class;
> @@ -133,7 +134,7 @@ static __maybe_unused int value_set_##_name(struct ctf_writer *cw,	\
>  			     const char *name,				\
>  			     _val_type val)				\
>  {									\
> -	struct bt_ctf_field_type *type = cw->data._name;		\
> +	struct bt_ctf_field_type *type = cw->data.fts._name;		\
>  	return value_set(type, event, name, (u64) val);			\
>  }
>  
> @@ -150,7 +151,7 @@ static __maybe_unused int
>  value_set_string(struct ctf_writer *cw, struct bt_ctf_event *event,
>  		 const char *name, const char *string)
>  {
> -	struct bt_ctf_field_type *type = cw->data.string;
> +	struct bt_ctf_field_type *type = cw->data.fts.string;
>  	struct bt_ctf_field *field;
>  	int ret = 0;
>  
> @@ -181,25 +182,25 @@ get_tracepoint_field_type(struct ctf_writer *cw, struct format_field *field)
>  	unsigned long flags = field->flags;
>  
>  	if (flags & FIELD_IS_STRING)
> -		return cw->data.string;
> +		return cw->data.fts.string;
>  
>  	if (!(flags & FIELD_IS_SIGNED)) {
>  		/* unsigned long are mostly pointers */
>  		if (flags & FIELD_IS_LONG || flags & FIELD_IS_POINTER)
> -			return cw->data.u64_hex;
> +			return cw->data.fts.u64_hex;
>  	}
>  
>  	if (flags & FIELD_IS_SIGNED) {
>  		if (field->size == 8)
> -			return cw->data.s64;
> +			return cw->data.fts.s64;
>  		else
> -			return cw->data.s32;
> +			return cw->data.fts.s32;
>  	}
>  
>  	if (field->size == 8)
> -		return cw->data.u64;
> +		return cw->data.fts.u64;
>  	else
> -		return cw->data.u32;
> +		return cw->data.fts.u32;
>  }
>  
>  static unsigned long long adjust_signedness(unsigned long long value_int, int size)
> @@ -969,8 +970,8 @@ static int add_tracepoint_types(struct ctf_writer *cw,
>  static int add_bpf_output_types(struct ctf_writer *cw,
>  				struct bt_ctf_event_class *class)
>  {
> -	struct bt_ctf_field_type *len_type = cw->data.u32;
> -	struct bt_ctf_field_type *seq_base_type = cw->data.u32_hex;
> +	struct bt_ctf_field_type *len_type = cw->data.fts.u32;
> +	struct bt_ctf_field_type *seq_base_type = cw->data.fts.u32_hex;
>  	struct bt_ctf_field_type *seq_type;
>  	int ret;
>  
> @@ -1013,31 +1014,31 @@ static int add_generic_types(struct ctf_writer *cw, struct perf_evsel *evsel,
>  	} while (0)
>  
>  	if (type & PERF_SAMPLE_IP)
> -		ADD_FIELD(event_class, cw->data.u64_hex, "perf_ip");
> +		ADD_FIELD(event_class, cw->data.fts.u64_hex, "perf_ip");
>  
>  	if (type & PERF_SAMPLE_TID) {
> -		ADD_FIELD(event_class, cw->data.s32, "perf_tid");
> -		ADD_FIELD(event_class, cw->data.s32, "perf_pid");
> +		ADD_FIELD(event_class, cw->data.fts.s32, "perf_tid");
> +		ADD_FIELD(event_class, cw->data.fts.s32, "perf_pid");
>  	}
>  
>  	if ((type & PERF_SAMPLE_ID) ||
>  	    (type & PERF_SAMPLE_IDENTIFIER))
> -		ADD_FIELD(event_class, cw->data.u64, "perf_id");
> +		ADD_FIELD(event_class, cw->data.fts.u64, "perf_id");
>  
>  	if (type & PERF_SAMPLE_STREAM_ID)
> -		ADD_FIELD(event_class, cw->data.u64, "perf_stream_id");
> +		ADD_FIELD(event_class, cw->data.fts.u64, "perf_stream_id");
>  
>  	if (type & PERF_SAMPLE_PERIOD)
> -		ADD_FIELD(event_class, cw->data.u64, "perf_period");
> +		ADD_FIELD(event_class, cw->data.fts.u64, "perf_period");
>  
>  	if (type & PERF_SAMPLE_WEIGHT)
> -		ADD_FIELD(event_class, cw->data.u64, "perf_weight");
> +		ADD_FIELD(event_class, cw->data.fts.u64, "perf_weight");
>  
>  	if (type & PERF_SAMPLE_DATA_SRC)
> -		ADD_FIELD(event_class, cw->data.u64, "perf_data_src");
> +		ADD_FIELD(event_class, cw->data.fts.u64, "perf_data_src");
>  
>  	if (type & PERF_SAMPLE_TRANSACTION)
> -		ADD_FIELD(event_class, cw->data.u64, "perf_transaction");
> +		ADD_FIELD(event_class, cw->data.fts.u64, "perf_transaction");
>  
>  #undef ADD_FIELD
>  	return 0;
> @@ -1109,7 +1110,7 @@ static int setup_events(struct ctf_writer *cw, struct perf_session *session)
>  #define __NON_SAMPLE_ADD_FIELD(t, n)						\
>  	do {							\
>  		pr2("  field '%s'\n", #n);			\
> -		if (bt_ctf_event_class_add_field(event_class, cw->data.t, #n)) {\
> +		if (bt_ctf_event_class_add_field(event_class, cw->data.fts.t, #n)) {\
>  			pr_err("Failed to add field '%s';\n", #n);\
>  			return -1;				\
>  		}						\
> @@ -1324,15 +1325,15 @@ do {							\
>  		goto err;				\
>  } while (0)
>  
> -	CREATE_INT_TYPE(cw->data.s64, 64, true,  false);
> -	CREATE_INT_TYPE(cw->data.u64, 64, false, false);
> -	CREATE_INT_TYPE(cw->data.s32, 32, true,  false);
> -	CREATE_INT_TYPE(cw->data.u32, 32, false, false);
> -	CREATE_INT_TYPE(cw->data.u32_hex, 32, false, true);
> -	CREATE_INT_TYPE(cw->data.u64_hex, 64, false, true);
> +	CREATE_INT_TYPE(cw->data.fts.s64, 64, true,  false);
> +	CREATE_INT_TYPE(cw->data.fts.u64, 64, false, false);
> +	CREATE_INT_TYPE(cw->data.fts.s32, 32, true,  false);
> +	CREATE_INT_TYPE(cw->data.fts.u32, 32, false, false);
> +	CREATE_INT_TYPE(cw->data.fts.u32_hex, 32, false, true);
> +	CREATE_INT_TYPE(cw->data.fts.u64_hex, 64, false, true);
>  
> -	cw->data.string  = bt_ctf_field_type_string_create();
> -	if (cw->data.string)
> +	cw->data.fts.string  = bt_ctf_field_type_string_create();
> +	if (cw->data.fts.string)
>  		return 0;
>  
>  err:
> @@ -1406,7 +1407,7 @@ static int ctf_writer__init(struct ctf_writer *cw, const char *path)
>  	if (!pkt_ctx_type)
>  		goto err_cleanup;
>  
> -	ret = bt_ctf_field_type_structure_add_field(pkt_ctx_type, cw->data.u32, "cpu_id");
> +	ret = bt_ctf_field_type_structure_add_field(pkt_ctx_type, cw->data.fts.u32, "cpu_id");
>  	bt_ctf_field_type_put(pkt_ctx_type);
>  	if (ret)
>  		goto err_cleanup;
> -- 
> 2.9.3
> 

  reply	other threads:[~2016-10-24 21:08 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-24 19:23 [PATCH] perf data convert: fix possible leak Philippe Proulx
2016-10-24 21:08 ` Jiri Olsa [this message]
  -- strict thread matches above, loose matches on Subject: below --
2016-02-11 22:46 Philippe Proulx

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20161024210841.GA2382@krava \
    --to=jolsa@redhat.com \
    --cc=ast@kernel.org \
    --cc=brendan.d.gregg@gmail.com \
    --cc=davem@davemloft.net \
    --cc=eeppeliteloop@gmail.com \
    --cc=jeremie.galarneau@gmail.com \
    --cc=jolsa@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lizefan@huawei.com \
    --cc=masami.hiramatsu.pt@hitachi.com \
    --cc=namhyung@kernel.org \
    --cc=pi3orama@163.com \
    --cc=wangnan0@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox