Re: Re: [PATCH V2 1/3] trace-cmd: Define general functions foroutputting/inputting saved_cmdlines

[Yoshihiro YUNOMAE]

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@xxxxxxxxxxx>
> > ---
> >   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@xxxxxxxxxxx


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/