On Tue, 7 Aug 2018 19:00:13 +0300
"Yordan Karadzhov (VMware)" <y.karadz@xxxxxxxxx> wrote:
> Add to the C API of KernelShark instruments for saving/loading of
> filter configuration to/from Json files.
>
> Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@xxxxxxxxx>
> ---
> kernel-shark-qt/src/CMakeLists.txt | 1 +
> kernel-shark-qt/src/libkshark-json.c | 601 +++++++++++++++++++++++++++
> kernel-shark-qt/src/libkshark.h | 49 +++
> 3 files changed, 651 insertions(+)
> create mode 100644 kernel-shark-qt/src/libkshark-json.c
>
> diff --git a/kernel-shark-qt/src/CMakeLists.txt b/kernel-shark-qt/src/CMakeLists.txt
> index ea5dbda..b3de765 100644
> --- a/kernel-shark-qt/src/CMakeLists.txt
> +++ b/kernel-shark-qt/src/CMakeLists.txt
> @@ -2,6 +2,7 @@ message("\n src ...")
>
> message(STATUS "libkshark")
> add_library(kshark SHARED libkshark.c
> + libkshark-json.c
> libkshark-model.c
> libkshark-collection.c)
>
> diff --git a/kernel-shark-qt/src/libkshark-json.c b/kernel-shark-qt/src/libkshark-json.c
> new file mode 100644
> index 0000000..d06488e
> --- /dev/null
> +++ b/kernel-shark-qt/src/libkshark-json.c
> @@ -0,0 +1,601 @@
> +// SPDX-License-Identifier: LGPL-2.1
> +
> +/*
> + * Copyright (C) 2018 VMware Inc, Yordan Karadzhov <y.karadz@xxxxxxxxx>
> + */
> +
> + /**
> + * @file libkshark-json.c
> + * @brief Json Confoguration I/O.
> + */
> +
> +// C
> +/** Use GNU C Library. */
> +#define _GNU_SOURCE
> +#include <stdio.h>
> +
> +// KernelShark
> +#include "libkshark.h"
> +
> +/**
> + * @brief Create an empty Json document and set its type description.
> + *
> + * @param type: String describing the type of the document,
> + * e.g. "kshark.record.config" or "kshark.filter.config".
> + *
> + * @returns json_object instance. Use json_object_put() to free the object.
I wonder if we should add handlers like:
kshark_config_free(struct json_object *jobj);
and call that instead? Perhaps even create our own object that may
contain extra state that the json object does not, and return that?
> + */
> +struct json_object *kshark_config_alloc(const char *type)
> +{
> + json_object *jobj, *jtype;
> +
> + jobj = json_object_new_object();
> + jtype = json_object_new_string(type);
> +
> + if (!jobj || !jtype)
> + goto fail;
> +
> + /* Set the type of this Json document. */
> + json_object_object_add(jobj, "type", jtype);
> +
> + return jobj;
> +
> + fail:
> + fprintf(stderr, "Failed to allocate memory for json_object.\n");
> + json_object_put(jobj);
> + json_object_put(jtype);
> +
> + return NULL;
> +}
> +
> +/**
> + * @brief Create an empty Record Configuration document. The type description
> + * is set to "kshark.record.config".
> + *
> + * @returns json_object instance. Use json_object_put() to free the object.
> + */
> +struct json_object *kshark_record_config_alloc()
> +{
> + return kshark_config_alloc("kshark.record.config");
> +}
> +
> +/**
> + * @brief Create an empty Filter Configuration document. The type description
> + * is set to "kshark.filter.config".
> + *
> + * @returns json_object instance. Use json_object_put() to free the object.
> + */
> +struct json_object *kshark_filter_config_alloc()
> +{
> + return kshark_config_alloc("kshark.filter.config");;
> +}
> +
> +/**
> + * @brief Record the current configuration of an Event Id filter into a Json
> + * document.
> + *
> + * @param pevt: Input location for the Page event.
> + * @param filter: Input location for an Id filter.
> + * @param filter_name: The name of the filter to show up in the Json document.
> + * @param jobj: Input location for the json_object instance.
> + */
> +void kshark_event_filter_to_json(struct pevent *pevt,
Please call "struct pevent" variables "pevent". Need to stay
consistent. Ideally, we should pick a single name for types of data
structures. This will be useful when we rename pevent to something else
as well.
> + struct tracecmd_filter_id *filter,
> + const char *filter_name,
> + struct json_object *jobj)
> +{
> + json_object *jfilter_data, *jevent, *jsystem, *jname;
> + int i, evt, *ids;
> + char *temp;
> +
> + /* Get the array of Ids to be fitered. */
> + ids = tracecmd_filter_ids(filter);
ids needs to be freed. And tracecmd_filter_ids() needs documentation
describing it :-/ (that's my fault, hmm Tzvetomir could fix this ;-).
It also needs to be checked for NULL.
> +
> + /* Create a Json array and fill the Id values into it. */
> + jfilter_data = json_object_new_array();
> + if (!jfilter_data)
> + goto fail;
> +
> + for (i = 0; i < filter->count; ++i) {
> + for (evt = 0; evt < pevt->nr_events; ++evt) {
> + if (pevt->events[evt]->id == ids[i]) {
> + jevent = json_object_new_object();
> +
> + temp = pevt->events[evt]->system;
> + jsystem = json_object_new_string(temp);
> +
> + temp = pevt->events[evt]->name;
> + jname = json_object_new_string(temp);
> +
> + if (!jevent || !jsystem || !jname)
> + goto fail;
> +
> + json_object_object_add(jevent, "system",
> + jsystem);
> +
> + json_object_object_add(jevent, "name",
> + jname);
> +
> + json_object_array_add(jfilter_data, jevent);
> +
> + break;
> + }
> + }
> + }
> +
> + /* Add the array of Ids to the filter config document. */
> + json_object_object_add(jobj, filter_name, jfilter_data);
> +
> + return;
> +
> + fail:
> + fprintf(stderr, "Failed to allocate memory for json_object.\n");
> + json_object_put(jfilter_data);
> + json_object_put(jevent);
> + json_object_put(jsystem);
> + json_object_put(jname);
> +}
> +
> +/**
> + * @brief Load from Json document the configuration of an Event Id filter.
> + *
> + * @param pevt: Input location for the Page event.
> + * @param filter: Input location for an Id filter.
> + * @param filter_name: The name of the filter as showing up in the Json
> + * document.
> + * @param jobj: Input location for the json_object instance.
> + *
> + * @returns True, if a filter has been loaded. If the filter configuration
> + * document contains no data for this particular filter or in a case
> + * of an error, the function returns False.
> + */
> +bool kshark_event_filter_from_json(struct pevent *pevt,
> + struct tracecmd_filter_id *filter,
> + const char *filter_name,
> + struct json_object *jobj)
> +{
> + json_object *jfilter, *jevent, *jsystem, *jname;
> + const char *system_str, *name_str;
> + struct event_format *event;
> + int i, length;
> +
> + /*
> + * Use the name of the filter to find the array of events associated
> + * with this filter. Notice that the filter config document may
> + * contain no data for this particular filter.
> + */
> + json_object_object_get_ex(jobj, filter_name, &jfilter);
Shouldn't you add a check of the return value of the above. Looking at
the documentation:
http://json-c.github.io/json-c/json-c-0.12/doc/html/json__object_8h.html#af3f38b3395b1af8e9d3ac73818c3a936
You could have:
ret = json_object_object_get_ex(jobj, filter_name, &jfilter);
if (!ret || json_object_get_type(jfilter) != ...
> + if (!jfilter || json_object_get_type(jfilter) != json_type_array)
> + return false;
> +
> + /* Set the filter. */
> + length = json_object_array_length(jfilter);
> + for (i = 0; i < length; ++i) {
> + jevent = json_object_array_get_idx(jfilter, i);
> +
> + json_object_object_get_ex(jevent, "system", &jsystem);
> + json_object_object_get_ex(jevent, "name", &jname);
Same here.
> + if (!jsystem || !jname)
> + goto fail;
> +
> + system_str = json_object_get_string(jsystem);
> + name_str = json_object_get_string(jname);
> +
> + event = pevent_find_event_by_name(pevt, system_str, name_str);
> + if (!event)
> + goto fail;
> +
> + tracecmd_filter_id_add(filter, event->id);
> + }
> +
> + return true;
> +
> + fail:
> + fprintf(stderr, "Failed to load event filter from json_object.\n");
> + return false;
> +}
> +
> +static void kshark_task_filter_to_json(struct tracecmd_filter_id *filter,
> + const char *filter_name,
> + struct json_object *jobj)
> +{
> + json_object *jfilter_data, *jpid;
> + int i, *ids;
> +
> + /* Get the array of Ids to be fitered. */
> + ids = tracecmd_filter_ids(filter);
Again, ids need to be freed.
> +
> + /* Create a Json array and fill the Id values into it. */
> + jfilter_data = json_object_new_array();
> + if (!jfilter_data)
Hmm, can json_object_put() take a NULL pointer?
> + goto fail;
> +
> + for (i = 0; i < filter->count; ++i) {
> + jpid = json_object_new_int(ids[i]);
> + if (!jpid)
> + goto fail;
> +
> + json_object_array_add(jfilter_data, jpid);
> + }
> +
> + /* Add the array of Ids to the filter config document. */
> + json_object_object_add(jobj, filter_name, jfilter_data);
> +
> + return;
> +
> + fail:
> + fprintf(stderr, "Failed to allocate memory for json_object.\n");
> + json_object_put(jfilter_data);
> + json_object_put(jpid);
> +}
> +
> +static bool kshark_task_filter_from_json(struct tracecmd_filter_id *filter,
> + const char *filter_name,
> + struct json_object *jobj)
> +{
> + json_object *jfilter, *jpid;
> + int i, length;
> +
> + /*
> + * Use the name of the filter to find the array of events associated
> + * with this filter. Notice that the filter config document may
> + * contain no data for this particular filter.
> + */
> + json_object_object_get_ex(jobj, filter_name, &jfilter);
> + if (!jfilter || json_object_get_type(jfilter) != json_type_array)
> + return false;
> +
> + /* Set the filter. */
> + length = json_object_array_length(jfilter);
> + for (i = 0; i < length; ++i) {
> + jpid = json_object_array_get_idx(jfilter, i);
> + if (!jpid)
> + goto fail;
> +
> + tracecmd_filter_id_add(filter, json_object_get_int(jpid));
> + }
> +
> + return true;
> +
> + fail:
> + fprintf(stderr, "Failed to load task filter from json_object.\n");
> + return false;
> +}
> +
> +/**
> + * @brief Record the current configuration of the advanced filter into a Json
> + * document.
> + *
> + * @param kshark_ctx: Input location for session context pointer.
> + * @param jobj: Input location for the json_object instance.
> + */
> +void kshark_adv_filters_to_json(struct kshark_context *kshark_ctx,
> + struct json_object *jobj)
> +{
> + struct event_filter *adv_filter = kshark_ctx->advanced_event_filter;
> + json_object *jfilter_data, *jevent, *jsystem, *jname, *jfilter;
> + struct event_format **events;
> + char *str;
> + int i;
> +
> + if (!kshark_ctx->advanced_event_filter->filters)
Should we also check if advanced_event_filter is NULL?
> + return;
> +
> + /* Create a Json array and fill the Id values into it. */
> + jfilter_data = json_object_new_array();
> + if (!jfilter_data)
> + goto fail;
> +
> + events = pevent_list_events(kshark_ctx->pevent, EVENT_SORT_SYSTEM);
events returned could be NULL. Need to check that.
(will review the rest tomorrow)
-- Steve
> +
> + for (i = 0; events[i]; i++) {
> + str = pevent_filter_make_string(adv_filter,
> + events[i]->id);
> + if (!str)
> + continue;
> +
> + jevent = json_object_new_object();
> + jsystem = json_object_new_string(events[i]->system);
> + jname = json_object_new_string(events[i]->name);
> + jfilter = json_object_new_string(str);
> + if (!jevent || !jsystem || !jname || !jfilter)
> + goto fail;
> +
> + json_object_object_add(jevent, "system", jsystem);
> + json_object_object_add(jevent, "name", jname);
> + json_object_object_add(jevent, "condition", jfilter);
> +
> + json_object_array_add(jfilter_data, jevent);
> + }
> +
> + /* Add the array of advanced filters to the filter config document. */
> + json_object_object_add(jobj, "adv event filter", jfilter_data);
> +
> + return;
> +
|