On Wed, 7 Jul 2021 11:12:03 +0300
"Yordan Karadzhov (VMware)" <y.karadz@xxxxxxxxx> wrote:
> Hi Steven,
> > +
> > + switch (cmd) {
> > + case START:
> > + ret = tracefs_hist_start(hist);
> > + if (ret) {
> > + char *err = tracefs_error_last(instance);
> > + if (err)
> > + fprintf(stderr, "\n%s\n", err);
> > + }
> > + break;
> > + case PAUSE:
> > + ret = tracefs_hist_pause(hist);
> > + break;
> > + case CONT:
> > + ret = tracefs_hist_continue(hist);
> > + break;
> > + case RESET:
> > + ret = tracefs_hist_reset(hist);
> > + break;
> > + case DELETE:
> > + ret = tracefs_hist_destroy(hist);
> > + break;
> > + case SHOW: {
> > + char *content;
> > + content = tracefs_event_file_read(instance, "kmem", "kmalloc",
> > + "hist", NULL);
>
> It looks more intuitive to have
>
> char *tracefs_hist_read(struct tracefs_hist *hist)
I thought a little about this, and rather have parsing of the file. We
could always add a helper function to do this later if it is helpful,
>
> > + ret = content ? 0 : -1;
> > + if (content) {
> > + printf("%s\n", content);
> > + free(content);
> > + }
> > + break;
> > + }
> > + }
>
> This "switch" can move to the library as well.
> We can have a method
>
> int tracefs_hist_ctrl(struct tracefs_hist *hist,
> const char *cmd,
> void *output);
>
Both this and the read file above could be added as enhancements, but I
don't think we need to add them to this current patch.
Thanks,
-- Steve
|