[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <5175E39F.4020706@hitachi.com>
Date: Tue, 23 Apr 2013 10:27:59 +0900
From: Yoshihiro YUNOMAE <yoshihiro.yunomae.ez@...achi.com>
To: Steven Rostedt <rostedt@...dmis.org>
Cc: Hidehiro Kawai <hidehiro.kawai.ez@...achi.com>,
Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>,
linux-kernel@...r.kernel.org, yrl.pp-manager.tt@...achi.com
Subject: Re: Re: [PATCH V2 1/3] trace-cmd: Define general functions for
outputting/inputting saved_cmdlines
Hi Steven,
(2013/04/23 5:06), Steven Rostedt wrote:
> On Mon, 2013-04-22 at 18:43 +0900, Yoshihiro YUNOMAE wrote:
>> Currently, trace-cmd outputs data of saved_cmdlines to a trace.dat file
>> in create_file_fd() and inputs the data from the file in tracecmd_init_data().
>> On the other hand, trace-cmd will also output and input data of trace_clock in
>> those functions in the patch "trace-cmd: Add recording to trace_clock" and
>> "Add support for extracting trace_clock in report".
>>
>> The source code of the output/input of saved_cmdlines data can be reused when
>> extract trace_clock, so we define general functions for outputting/inputting a
>> file on debugfs.
>>
>> Signed-off-by: Yoshihiro YUNOMAE <yoshihiro.yunomae.ez@...achi.com>
>> ---
>> trace-input.c | 46 +++++++++++++++++++++++++++++-------------
>> trace-output.c | 62 ++++++++++++++++++++++++++++++++------------------------
>> 2 files changed, 67 insertions(+), 41 deletions(-)
>>
>> diff --git a/trace-input.c b/trace-input.c
>> index 56a8e8d..232015a 100644
>> --- a/trace-input.c
>> +++ b/trace-input.c
>> @@ -1870,6 +1870,37 @@ static int read_cpu_data(struct tracecmd_input *handle)
>>
>> }
>>
>> +static int read_data_and_size(struct tracecmd_input *handle,
>> + char **data, unsigned long long *size)
>> +{
>> + *size = read8(handle);
>> + if (*size < 0)
>> + return -1;
>> + *data = malloc(*size + 1);
>> + if (!*data)
>> + return -1;
>> + if (do_read_check(handle, *data, *size)) {
>> + free(*data);
>> + return -1;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int read_and_parse_cmdlines(struct tracecmd_input *handle,
>> + struct pevent *pevent)
>> +{
>> + unsigned long long size;
>> + char *cmdlines;
>> +
>> + if (read_data_and_size(handle, &cmdlines, &size) < 0)
>> + return -1;
>> + cmdlines[size] = 0;
>> + parse_cmdlines(pevent, cmdlines, size);
>> + free(cmdlines);
>> + return 0;
>> +}
>> +
>> /**
>> * tracecmd_init_data - prepare reading the data from trace.dat
>> * @handle: input handle for the trace.dat file
>> @@ -1880,23 +1911,10 @@ static int read_cpu_data(struct tracecmd_input *handle)
>> int tracecmd_init_data(struct tracecmd_input *handle)
>> {
>> struct pevent *pevent = handle->pevent;
>> - unsigned long long size;
>> - char *cmdlines;
>> int ret;
>>
>> - size = read8(handle);
>> - if (size < 0)
>> - return -1;
>> - cmdlines = malloc(size + 1);
>> - if (!cmdlines)
>> + if (read_and_parse_cmdlines(handle, pevent) < 0)
>> return -1;
>> - if (do_read_check(handle, cmdlines, size)) {
>> - free(cmdlines);
>> - return -1;
>> - }
>> - cmdlines[size] = 0;
>> - parse_cmdlines(pevent, cmdlines, size);
>> - free(cmdlines);
>>
>> handle->cpus = read4(handle);
>> if (handle->cpus < 0)
>> diff --git a/trace-output.c b/trace-output.c
>> index 460b773..8697976 100644
>> --- a/trace-output.c
>> +++ b/trace-output.c
>> @@ -687,6 +687,39 @@ static int read_ftrace_printk(struct tracecmd_output *handle)
>> return -1;
>> }
>>
>> +static int save_tracing_file_data(struct tracecmd_output *handle,
>> + const char *filename)
>> +{
>> + unsigned long long endian8;
>> + char *file = NULL;
>> + struct stat st;
>> + off64_t check_size;
>> + off64_t size;
>> + int ret;
>> +
>> + file = get_tracing_file(handle, filename);
>> + ret = stat(file, &st);
>> + if (ret >= 0) {
>> + size = get_size(file);
>> + endian8 = convert_endian_8(handle, size);
>> + if (do_write_check(handle, &endian8, 8))
>> + return -1;
>
> Can't do a return here. The "get_tracing_file()" does an allocation that
> needs to be freed with "put_tracing_file()" below. You still need the
> goto out_free, or something.
Oh, I see.
I'll send a revised patch set soon.
Thanks,
Yoshihiro YUNOMAE
> You can initialize ret = -1; and then below have:
>
>> + check_size = copy_file(handle, file);
>> + if (size != check_size) {
>> + errno = EINVAL;
>> + warning("error in size of file '%s'", file);
>> + return -1;
>> + }
>> + } else {
>> + size = 0;
>> + endian8 = convert_endian_8(handle, size);
>> + if (do_write_check(handle, &endian8, 8))
>> + return -1;
>> + }
>
> ret = 0;
> out_free:
>
> -- Steve
>
>> + put_tracing_file(file);
>> + return 0;
>> +}
>> +
>> static struct tracecmd_output *
>> create_file_fd(int fd, struct tracecmd_input *ihandle,
>> const char *tracing_dir,
--
Yoshihiro YUNOMAE
Software Platform Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: yoshihiro.yunomae.ez@...achi.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists