Re: [PATCH] building libtraceevent with clang

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

Em Fri, Feb 10, 2017 at 02:14:08PM -0500, Steven Rostedt escreveu:
> On Fri, 10 Feb 2017 14:03:17 -0300
> Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> > +	case OLD_RINGBUF_TYPE_TIME_EXTEND:
> > +		extend = read_4(kbuf, ptr);
> > +		extend <<= TS_SHIFT;
> > +		extend += delta;
> > +		delta = extend;
> > +		ptr += 4;
> > +		/* Fall through */
 
> No, actually, the length should be zeroed here.
 
> 	length = 0;
 
> As the ptr is placed passed the metadata. At the end of this case
> statement, the ptr will be after the metadata. The length is
> to represent the length of the data that is part of the event. The
> index is the ptr - start_data + length. As there's no data to this type
> of event, it should be zero.
 
> When I add length = 0, I get more events from my old 2.6.30 trace.dat
> file :-/  I'll have to update my tests.
 
> Care to send another patch?

I have this now in my tree, and since the patch is basically yours, I
just made myself the reporter and you the author, please ack.


commit 1fb82b6cba967afde11957099a9ef440cf8c2248
Author: Steven Rostedt <rostedt@goodmis.org>
Date:   Mon Feb 13 13:18:29 2017 -0300

    tools lib traceevent: Initialize lenght on OLD_RING_BUFFER_TYPE_TIME_STAMP
    
    A undefined value was being used for the OLD_RING_BUFFER_TYPE_TIME_STAMP
    case entry, as the 'lenght' variable was not being initialized, fix it.
    
    Caught by the reporter when building tools/perf/ using clang, which emmitted
    this warning:
    
      kbuffer-parse.c:312:7: warning: variable 'length' is used uninitialized whenever switch case is taken [-Wsometimes-uninitialized]
              case OLD_RINGBUF_TYPE_TIME_EXTEND:
                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
      kbuffer-parse.c:339:29: note: uninitialized use occurs here
              kbuf->next = kbuf->index + length;
                                       ^~~~~~
      kbuffer-parse.c:297:21: note: initialize the variable 'length' to silence this warning
              unsigned int length;
                                 ^
                                  = 0
    
    Reported-by: Arnaldo Carvalho de Melo <acme@redhat.com>
    Cc: Adrian Hunter <adrian.hunter@intel.com>
    Cc: David Ahern <dsahern@gmail.com>
    Cc: Jiri Olsa <jolsa@kernel.org>
    Cc: Namhyung Kim <namhyung@kernel.org>
    Cc: Wang Nan <wangnan0@huawei.com>
    Link: http://lkml.kernel.org/r/20170210141408.5eeb6e91@gandalf.local.home
    Signed-off-by: Steven Rostedt <rostedt@goodmis.org>

diff --git a/tools/lib/traceevent/kbuffer-parse.c b/tools/lib/traceevent/kbuffer-parse.c
index 65984f1c2974..2009cb7d9675 100644
--- a/tools/lib/traceevent/kbuffer-parse.c
+++ b/tools/lib/traceevent/kbuffer-parse.c
@@ -315,6 +315,7 @@ static unsigned int old_update_pointers(struct kbuffer *kbuf)
 		extend += delta;
 		delta = extend;
 		ptr += 4;
+		lenght = 0;
 		break;
 
 	case OLD_RINGBUF_TYPE_TIME_STAMP: