linux-trace-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Steven Rostedt <rostedt@goodmis.org>
To: "Yordan Karadzhov (VMware)" <y.karadz@gmail.com>
Cc: linux-trace-devel@vger.kernel.org
Subject: Re: [PATCH 6/7] kernel-shark: Check if "trace_seq" was destroyed before using it
Date: Fri, 14 May 2021 09:31:33 -0400	[thread overview]
Message-ID: <20210514093133.504d1008@gandalf.local.home> (raw)
In-Reply-To: <20210514121826.161749-7-y.karadz@gmail.com>

On Fri, 14 May 2021 15:18:25 +0300
"Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:

> When closing a "tep" data stream we destroy the "trace_seq" object.
> However, trace_seq_destroy() sets the buffer to "TRACE_SEQ_POISON"
> which is different from NULL.
> 
> It is unfortunate that TRACE_SEQ_POISON is an internal definition
> of libtraceevent, so we have to redefine it here, but this can be
> fixed in the future.

It's not unfortunate. It can change in the future without breaking API.

Redefining it here is not robust, and if trace-seq decides to do something
different with that poison value, this will break, and it can't be blamed
on API (using internal knowledge to implement code is not protected by
being backward compatible).

The correct solution is to NULL the buffer after calling destroy.

	if (seq.buffer) {
		trace_seq_destroy(&seq);
		seq.buffer = NULL;
	}

Just like you would do with a pointer you free but may use NULL as a value
you are checking.

-- Steve


> 
> Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
> ---
>  src/libkshark-tepdata.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/src/libkshark-tepdata.c b/src/libkshark-tepdata.c
> index bc5babb..4a84141 100644
> --- a/src/libkshark-tepdata.c
> +++ b/src/libkshark-tepdata.c
> @@ -29,11 +29,17 @@
>  #include "libkshark-plugin.h"
>  #include "libkshark-tepdata.h"
>  
> +/**
> + * The TEP_SEQ_POISON is to catch the use of
> + * a trace_seq structure after it was destroyed.
> + */
> +#define TEP_SEQ_POISON	((void *)0xdeadbeef)
> +
>  static __thread struct trace_seq seq;
>  
>  static bool init_thread_seq(void)
>  {
> -	if (!seq.buffer)
> +	if (!seq.buffer || seq.buffer == TEP_SEQ_POISON)
>  		trace_seq_init(&seq);
>  
>  	return seq.buffer != NULL;


  reply	other threads:[~2021-05-14 13:31 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-14 12:18 [PATCH 0/7] Final fixes before KS 2.0 Yordan Karadzhov (VMware)
2021-05-14 12:18 ` [PATCH 1/7] kernel-shark: Preserve markers when appending data Yordan Karadzhov (VMware)
2021-05-14 12:18 ` [PATCH 2/7] kernel-shark: Preserve open graphs " Yordan Karadzhov (VMware)
2021-05-14 12:18 ` [PATCH 3/7] kernel-shark: Clear before loading new session Yordan Karadzhov (VMware)
2021-05-14 12:18 ` [PATCH 4/7] kernel-shark: Better handling of plugins when appending data file Yordan Karadzhov (VMware)
2021-05-14 12:18 ` [PATCH 5/7] kernel-shark: Do draw the combo point of the mark Yordan Karadzhov (VMware)
2021-05-14 12:18 ` [PATCH 6/7] kernel-shark: Check if "trace_seq" was destroyed before using it Yordan Karadzhov (VMware)
2021-05-14 13:31   ` Steven Rostedt [this message]
2021-05-14 13:45     ` Yordan Karadzhov
2021-05-14 13:57       ` Steven Rostedt
2021-05-14 14:01         ` Yordan Karadzhov
2021-05-14 12:18 ` [PATCH 7/7] kernel-shark: Quiet the warning printing from libtraceevent Yordan Karadzhov (VMware)
2021-05-14 13:33   ` Steven Rostedt
2021-05-14 17:45 ` [PATCH 0/7] Final fixes before KS 2.0 Steven Rostedt
2021-05-14 18:01 ` Steven Rostedt
2021-05-17 10:22   ` Yordan Karadzhov
2021-05-17 12:54     ` Steven Rostedt
2021-05-17 13:04       ` Yordan Karadzhov

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=20210514093133.504d1008@gandalf.local.home \
    --to=rostedt@goodmis.org \
    --cc=linux-trace-devel@vger.kernel.org \
    --cc=y.karadz@gmail.com \
    --subject='Re: [PATCH 6/7] kernel-shark: Check if "trace_seq" was destroyed before using it' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).