From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932143AbdBJTOT (ORCPT ); Fri, 10 Feb 2017 14:14:19 -0500 Received: from mail.kernel.org ([198.145.29.136]:38712 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752385AbdBJTOR (ORCPT ); Fri, 10 Feb 2017 14:14:17 -0500 Date: Fri, 10 Feb 2017 14:14:08 -0500 From: Steven Rostedt To: Arnaldo Carvalho de Melo Cc: Namhyung Kim , Jiri Olsa , Linux Kernel Mailing List Subject: Re: [PATCH] building libtraceevent with clang Message-ID: <20170210141408.5eeb6e91@gandalf.local.home> In-Reply-To: <20170210170317.GA14313@kernel.org> References: <20170210170317.GA14313@kernel.org> X-Mailer: Claws Mail 3.14.0 (GTK+ 2.24.31; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, 10 Feb 2017 14:03:17 -0300 Arnaldo Carvalho de Melo wrote: > Hi Steven, > > I tried building perf (and thus libtraceevent) with clang and > got this one: > > 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 > 1 warning generated. Hmm, I'm surprised gcc never complained. > > > Please take a look if the following is what should be done: > > diff --git a/tools/lib/traceevent/kbuffer-parse.c b/tools/lib/traceevent/kbuffer-parse.c > index 65984f1c2974..3f717294cb82 100644 > --- a/tools/lib/traceevent/kbuffer-parse.c > +++ b/tools/lib/traceevent/kbuffer-parse.c > @@ -309,20 +309,20 @@ static unsigned int old_update_pointers(struct kbuffer *kbuf) > kbuf->next = kbuf->size; > return 0; > > - case OLD_RINGBUF_TYPE_TIME_EXTEND: > - extend = read_4(kbuf, ptr); > - extend <<= TS_SHIFT; > - extend += delta; > - delta = extend; > - ptr += 4; > - break; > - > case OLD_RINGBUF_TYPE_TIME_STAMP: > /* should never happen! */ > kbuf->curr = kbuf->size; > kbuf->next = kbuf->size; > kbuf->index = kbuf->size; > return -1; > + > + 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? -- Steve > default: > if (len) > length = len * 4;