[PATCH 0/3 V2] trace-cmd: compiler warning fixes
Darren Hart
mostly printf format type issues. Also includes an uninitialized
variable fix.
V2: actually send my patches and not 2 of rostedts...
--
Darren Hart
IBM Linux Technology Center
Real-Time Linux Team
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Darren Hart
---
trace-read.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/trace-read.c b/trace-read.c
index a04c85b..5befaba 100644
--- a/trace-read.c
+++ b/trace-read.c
@@ -216,7 +216,7 @@ static void read_rest(void)
r = read(input_fd, buf, BUFSIZ);
if (r > 0) {
buf[r] = 0;
- printf(buf);
+ printf("%s", buf);
}
} while (r > 0);
}
--
1.6.3.3
Darren Hart
You can pull these from the latest for-rostedt/master git tree:
git://git.kernel.org/pub/scm/linux/kernel/git/dvhart/trace-cmd.git for-rostedt/master
Darren Hart
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/kernel-shark.c b/kernel-shark.c
index 9dedf48..222381c 100644
--- a/kernel-shark.c
+++ b/kernel-shark.c
@@ -596,10 +596,10 @@ void kernel_shark(int argc, char **argv)
if (ret >= 0)
input_file = default_input_file;
}
- if (handle)
- handle = tracecmd_open(input_file);
+ handle = tracecmd_open(input_file);
- info->handle = handle;
+ if (handle)
+ info->handle = handle;
/* --- Main window --- */
--
1.6.3.3
Darren Hart
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/trace-graph.c b/trace-graph.c
index 135e516..1759163 100644
--- a/trace-graph.c
+++ b/trace-graph.c
@@ -886,13 +886,13 @@ static void draw_cpu_info(struct graph_info *ginfo, gint cpu, gint x, gint y)
trace_seq_init(&s);
- dprintf(3, "start=%zu end=%zu time=%lu\n", ginfo->start_time, ginfo->end_time, time);
+ dprintf(3, "start=%llu end=%llu time=%llu\n", ginfo->start_time, ginfo->end_time, time);
record = find_record_on_cpu(ginfo, cpu, time);
if (record) {
- dprintf(3, "record->ts=%llu time=%zu-%zu\n",
+ dprintf(3, "record->ts=%llu time=%llu-%llu\n",
record->ts, time, time-(gint)(1/ginfo->resolution));
print_rec_info(record, pevent, cpu);
--
1.6.3.3
John Kacur
> Signed-off-by: Darren Hart <dvh...@us.ibm.com>
> ---
> �kernel-shark.c | � �6 +++---
> �1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/kernel-shark.c b/kernel-shark.c
> index 9dedf48..222381c 100644
> --- a/kernel-shark.c
> +++ b/kernel-shark.c
> @@ -596,10 +596,10 @@ void kernel_shark(int argc, char **argv)
> � � � � � � � �if (ret >= 0)
> � � � � � � � � � � � �input_file = default_input_file;
> � � � �}
> - � � � if (handle)
> - � � � � � � � handle = tracecmd_open(input_file);
> + � � � handle = tracecmd_open(input_file);
>
> - � � � info->handle = handle;
> + � � � if (handle)
> + � � � � � � � info->handle = handle;
>
> � � � �/* --- Main window --- */
>
> --
This looks correct, but I'm wondering if it is safe to continue if the
call to tracecmd_open fails?
John Kacur
> ---
> �trace-read.c | � �2 +-
> �1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/trace-read.c b/trace-read.c
> index a04c85b..5befaba 100644
> --- a/trace-read.c
> +++ b/trace-read.c
> @@ -216,7 +216,7 @@ static void read_rest(void)
> � � � � � � � �r = read(input_fd, buf, BUFSIZ);
> � � � � � � � �if (r > 0) {
> � � � � � � � � � � � �buf[r] = 0;
> - � � � � � � � � � � � printf(buf);
> + � � � � � � � � � � � printf("%s", buf);
> � � � � � � � �}
> � � � �} while (r > 0);
> �}
> --
> 1.6.3.3
>
Oh! Obviously correct, thanks Darren.
Steven Rostedt
> andle)
> > > - handle = tracecmd_open(input_file);
> > > + handle = tracecmd_open(input_file);
> > >
> > > - info->handle = handle;
> > > + if (handle)
> > > + info->handle = handle;
> > >
> > > /* --- Main window --- */
> > >
> > > --
> >
> > This looks correct, but I'm wondering if it is safe to continue if the
> > call to tracecmd_open fails?
>
>
> - if (handle)
> + if (input_file)
Looking at the context, this isn't enough. We should have had:
if (input_file)
info->handle = tracecmd_open(input_file);
else
info->handle = NULL;
-- Steve
Steven Rostedt
> On Mon, Feb 1, 2010 at 5:50 PM, Darren Hart <dvh...@us.ibm.com> wrote:
> > Signed-off-by: Darren Hart <dvh...@us.ibm.com>
> > ---
> > kernel-shark.c | 6 +++---
> > 1 files changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/kernel-shark.c b/kernel-shark.c
> > index 9dedf48..222381c 100644
> > --- a/kernel-shark.c
> > +++ b/kernel-shark.c
> > @@ -596,10 +596,10 @@ void kernel_shark(int argc, char **argv)
> > if (ret >= 0)
> > input_file = default_input_file;
> > }
> > - if (handle)
> > - handle = tracecmd_open(input_file);
> > + handle = tracecmd_open(input_file);
> >
> > - info->handle = handle;
> > + if (handle)
> > + info->handle = handle;
> >
> > /* --- Main window --- */
> >
> > --
>
> This looks correct, but I'm wondering if it is safe to continue if the
> call to tracecmd_open fails?
Actually this patch is wrong. The real code should be:
- if (handle)
+ if (input_file)
-- Steve
Steven Rostedt
> On Mon, Feb 1, 2010 at 5:50 PM, Darren Hart <dvh...@us.ibm.com> wrote:
> > Signed-off-by: Darren Hart <dvh...@us.ibm.com>
> > ---
> > trace-read.c | 2 +-
> > 1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/trace-read.c b/trace-read.c
> > index a04c85b..5befaba 100644
> > --- a/trace-read.c
> > +++ b/trace-read.c
> > @@ -216,7 +216,7 @@ static void read_rest(void)
> > r = read(input_fd, buf, BUFSIZ);
> > if (r > 0) {
> > buf[r] = 0;
> > - printf(buf);
> > + printf("%s", buf);
> > }
> > } while (r > 0);
> > }
> > --
> > 1.6.3.3
> >
>
> Oh! Obviously correct, thanks Darren.
Of the three patches, I think this is the only one that is correct ;-)
-- Steve
John Kacur
> On Wed, 2010-02-03 at 11:17 -0500, Steven Rostedt wrote:
>> andle)
>> > > - � � � � � � � handle = tracecmd_open(input_file);
>> > > + � � � handle = tracecmd_open(input_file);
>> > >
>> > > - � � � info->handle = handle;
>> > > + � � � if (handle)
>> > > + � � � � � � � info->handle = handle;
>> > >
>> > > � � � �/* --- Main window --- */
>> > >
>> > > --
>> >
>> > This looks correct, but I'm wondering if it is safe to continue if the
>> > call to tracecmd_open fails?
>>
>> Actually this patch is wrong. The real code should be:
>>
>> - � � if (handle)
>> + � � if (input_file)
>
> Looking at the context, this isn't enough. We should have had:
>
> � � � �if (input_file)
> � � � � � � � �info->handle = tracecmd_open(input_file);
> � � � �else
> � � � � � � � �info->handle = NULL;
>
> -- Steve
>
Okay, are you going to push it to your repo for us? I would offer to
push it through mine if it would save you time, but it's probably
quicker if you just handle it.
John Kacur
> On Wed, 2010-02-03 at 17:07 +0100, John Kacur wrote:
>> On Mon, Feb 1, 2010 at 5:50 PM, Darren Hart <dvh...@us.ibm.com> wrote:
>> > Signed-off-by: Darren Hart <dvh...@us.ibm.com>
>> > ---
>> > �trace-read.c | � �2 +-
>> > �1 files changed, 1 insertions(+), 1 deletions(-)
>> >
>> > diff --git a/trace-read.c b/trace-read.c
>> > index a04c85b..5befaba 100644
>> > --- a/trace-read.c
>> > +++ b/trace-read.c
>> > @@ -216,7 +216,7 @@ static void read_rest(void)
>> > � � � � � � � �r = read(input_fd, buf, BUFSIZ);
>> > � � � � � � � �if (r > 0) {
>> > � � � � � � � � � � � �buf[r] = 0;
>> > - � � � � � � � � � � � printf(buf);
>> > + � � � � � � � � � � � printf("%s", buf);
>> > � � � � � � � �}
>> > � � � �} while (r > 0);
>> > �}
>> > --
>> > 1.6.3.3
>> >
>>
>> Oh! Obviously correct, thanks Darren.
>
> Of the three patches, I think this is the only one that is correct ;-)
>
Ah, you're a hard taskmaster! Are you going to push it to your repo
for us then pls?
Steven Rostedt
>
> Okay, are you going to push it to your repo for us? I would offer to
> push it through mine if it would save you time, but it's probably
> quicker if you just handle it.
Yeah, I'll pull via email the one patch and then do this one by hand.
-- Steve
Darren Hart
> On Wed, 2010-02-03 at 17:07 +0100, John Kacur wrote:
>> On Mon, Feb 1, 2010 at 5:50 PM, Darren Hart <dvh...@us.ibm.com> wrote:
>>> Signed-off-by: Darren Hart <dvh...@us.ibm.com>
>>> ---
>>> trace-read.c | 2 +-
>>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/trace-read.c b/trace-read.c
>>> index a04c85b..5befaba 100644
>>> --- a/trace-read.c
>>> +++ b/trace-read.c
>>> @@ -216,7 +216,7 @@ static void read_rest(void)
>>> r = read(input_fd, buf, BUFSIZ);
>>> if (r > 0) {
>>> buf[r] = 0;
>>> - printf(buf);
>>> + printf("%s", buf);
>>> }
>>> } while (r > 0);
>>> }
>>> --
>>> 1.6.3.3
>>>
>> Oh! Obviously correct, thanks Darren.
>
> Of the three patches, I think this is the only one that is correct ;-)
The other appear to depend on the guint arch specific implementation of
the guint64 type, so the only way to fix it with a cast to ull - or to
not use g types at all. Bleh.
--
Darren Hart
IBM Linux Technology Center
Real-Time Linux Team
Steven Rostedt
> > Of the three patches, I think this is the only one that is correct ;-)
>
> The other appear to depend on the guint arch specific implementation of
> the guint64 type, so the only way to fix it with a cast to ull - or to
> not use g types at all. Bleh.
I'm fixing it up by typecasting it to (u64), and defining it.
I probably should never have used guint64 but since that's the "glib"
thing to do, and when in Rome do as the Romans do, even if the Romans
are doing crap!
-- Steve