On Wed, 9 Aug 2023 13:53:33 -0400
Stevie Alvarez <stevie.6strings@xxxxxxxxx> wrote:
> From: Stevie Alvarez (Google) <stevie.6strings@xxxxxxxxx>
>
> Changes in v4:
> * Reorder enum traceeval_data_type numeric types for algorithmic access.
> * Reorder union traceeval_data numeric fields for legibility.
> * Remove parenthesis from function name in kerneldocs.
> * Optimize logic in type_alloc().
> * Distinguish internal vs public interfaces within include statements.
> * Rework copy_traceeval_data() logic for legibility.
> * Rework comparator return types to fix query and insert bugs.
> * update_entry() frees the entries old values to avoid a memory leak.
>
> Steven, I didn't get around to adding in the cstring field to
> traceeval_data, as I wasn't able to completely grasp what you were
> looking for. My apologies.
Thanks Stevie!
I'm going to pull these in as-is and build on top of them for the rest so
that you can work on other things. I'll still do a review of these patches
for your own understanding, but you don't need to send another series.
As for the cstring. Without it, I get these warnings in the task-eval.c program:
libtraceeval.git/samples/task-eval.c: In function ‘update_process’:
libtraceeval.git/samples/task-eval.c:197:35: warning: initialization discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
197 | .string = comm,
| ^~~~
libtraceeval.git/samples/task-eval.c: In function ‘get_process_data’:
libtraceeval.git/samples/task-eval.c:255:35: warning: initialization discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
255 | .string = comm,
| ^~~~
libtraceeval.git/samples/task-eval.c: In function ‘set_process_data’:
libtraceeval.git/samples/task-eval.c:278:35: warning: initialization discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
278 | .string = comm,
as in update_process() (and other functions) we have:
static void update_process(struct task_data *tdata, const char *comm,
enum sched_state state, enum command cmd,
unsigned long long ts)
{
const union traceeval_data keys[] = {
{
.string = comm,
},
{
.number = state,
},
};
Because these queries are performed on const char *strings. That is, in
order to do a traceeval_query() I need to populate the keys array. And if
I'm using a const char *string to do the search, I need to use a const
pointer to assign with.
So, if we supply a "const char * cstring" to the union, and I use:
const union traceeval_data keys[] = {
{
.cstring = comm,
},
It makes the compiler happy!
-- Steve
>
> ---
> v3 discussion: https://lore.kernel.org/linux-trace-devel/20230808180156.GA1300@3xKetch/T/#t
>
> Stevie Alvarez (Google) (5):
> histograms: Initial histograms interface
> histograms: Add traceeval initialize and release
> histograms: Add traceeval compare
> histograms: Add traceeval query
> histograms: Add traceeval insert
>
> Makefile | 2 +-
> include/traceeval-hist.h | 147 +++++++
> include/traceeval-test.h | 16 +
> src/Makefile | 1 +
> src/histograms.c | 804 +++++++++++++++++++++++++++++++++++++++
> 5 files changed, 969 insertions(+), 1 deletion(-)
> create mode 100644 include/traceeval-hist.h
> create mode 100644 include/traceeval-test.h
> create mode 100644 src/histograms.c
>
|