On Wed, Nov 24, 2021 at 6:08 AM Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:
>
> On Thu, 11 Nov 2021 17:07:30 +0200
> "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@xxxxxxxxx> wrote:
>
> > @@ -4437,6 +4456,30 @@ static void write_guest_file(struct buffer_instance *instance)
> > free(temp_files);
> > }
> >
> > +static struct tracecmd_output *create_output(struct common_record_context *ctx)
> > +{
> > + struct tracecmd_output *out;
> > + int fd;
> > +
> > + fd = open(ctx->output, O_RDWR | O_CREAT | O_TRUNC | O_LARGEFILE, 0644);
>
> I stopped at this patch because I really dislike the above.
>
> Why don't we have:
>
> tracecmd_output_allocate(file);
>
> and
> tracecmd_output_allocate_fd(fd);
>
> Where tracecmd_output_allocate(file) does:
>
> struct tracecmd_output *tracecmd_output_allocate(const char *file)
> {
> int fd;
>
> fd = open(file, O_RDWR | O_CREAT | O_TRUNC | O_LARGEFILE, 0644);
> if (fd < 0)
> return NULL;
> return tracecmd_output_allocate_fd(fd);
> }
>
> ?
>
> Then we could remove a lot of these duplicate opens all over the place.
>
> Although, I'm not sure I like the name allocate. It probably should be called:
>
> tracecmd_output_create();
>
> and we keep tracecmd_output_allocate() as is?
>
There is already such API, I'll replace that pattern with a call to this:
struct tracecmd_output *tracecmd_create_init_file(const char *output_file)
> -- Steve
>
>
> > + if (fd < 0)
> > + return NULL;
> > +
> > + out = tracecmd_output_allocate(fd);
> > + if (!out)
> > + goto error;
> > + if (tracecmd_output_write_headers(out, listed_events))
> > + goto error;
> > + return out;
> > +error:
> > + if (out)
--
Tzvetomir (Ceco) Stoyanov
VMware Open Source Technology Center
|