Re: [PATCH 4/4] libtracefs: Implement API to create / modify and display histograms

[Steven Rostedt <rostedt@goodmis.org>]

On Wed, 7 Jul 2021 11:12:03 +0300
"Yordan Karadzhov (VMware)" <y.karadz@gmail.com> 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