On 1/14/24 19:17, Benjamin ROBIN wrote:
Allocate a new kshark_config_doc only if the format is supported. Signed-off-by: Benjamin ROBIN <dev@xxxxxxxxxxxxx> --- src/libkshark-configio.c | 74 ++++++++++++++++++++++------------------ 1 file changed, 41 insertions(+), 33 deletions(-) diff --git a/src/libkshark-configio.c b/src/libkshark-configio.c index 9a1ba60..853e056 100644 --- a/src/libkshark-configio.c +++ b/src/libkshark-configio.c @@ -675,23 +675,25 @@ struct kshark_config_doc * kshark_export_plugin_file(struct kshark_plugin_list *plugin, enum kshark_config_formats format) { - /* Create a new Configuration document. */ - struct kshark_config_doc *conf = - kshark_config_new("kshark.config.library", format); - - if (!conf) - return NULL; + struct kshark_config_doc *conf;switch (format) {case KS_CONFIG_JSON: - kshark_plugin_to_json(plugin, conf->conf_doc); - return conf; + break;default:fprintf(stderr, "Document format %d not supported\n", - conf->format); + format); return NULL; } + + /* Create a new Configuration document. */ + conf = kshark_config_new("kshark.config.library", format); + if (!conf) + return NULL; + + kshark_plugin_to_json(plugin, conf->conf_doc); + return conf; }static bool kshark_all_plugins_to_json(struct kshark_context *kshark_ctx,@@ -739,22 +741,24 @@ struct kshark_config_doc * kshark_export_all_plugins(struct kshark_context *kshark_ctx, enum kshark_config_formats format) { - struct kshark_config_doc *conf = - kshark_config_new("kshark.config.plugins", KS_CONFIG_JSON); - - if (!conf) - return NULL; + struct kshark_config_doc *conf;switch (format) {case KS_CONFIG_JSON: - kshark_all_plugins_to_json(kshark_ctx, conf->conf_doc); - return conf;
I agree that this code is a bit confusing. Long time ago we had the idea that we will support not only 'json' but also other formats. And that we will have other 'kshark_plugin_to_xxx()' APIs. This is the reason to have this 'switch'. Please keep the code as it is and just call kshark_free_config_doc() in the default (error) case.
+ break;default:fprintf(stderr, "Document format %d not supported\n", - conf->format); + format); return NULL; } + + conf = kshark_config_new("kshark.config.plugins", format); + if (!conf) + return NULL; + + kshark_all_plugins_to_json(kshark_ctx, conf->conf_doc); + return conf; }static bool kshark_plugin_from_json(struct kshark_context *kshark_ctx,@@ -867,22 +871,24 @@ struct kshark_config_doc * kshark_export_stream_plugins(struct kshark_data_stream *stream, enum kshark_config_formats format) { - struct kshark_config_doc *conf = - kshark_config_new("kshark.config.plugins", KS_CONFIG_JSON); - - if (!conf) - return NULL; + struct kshark_config_doc *conf;switch (format) {case KS_CONFIG_JSON: - kshark_stream_plugins_to_json(stream, conf->conf_doc); - return conf;
Same comment applies here. Thanks! Y.
+ break;default:fprintf(stderr, "Document format %d not supported\n", - conf->format); + format); return NULL; } + + conf = kshark_config_new("kshark.config.plugins", KS_CONFIG_JSON); + if (!conf) + return NULL; + + kshark_stream_plugins_to_json(stream, conf->conf_doc); + return conf; }static bool kshark_stream_plugins_from_json(struct kshark_context *kshark_ctx,@@ -1019,23 +1025,25 @@ struct kshark_config_doc * kshark_export_model(struct kshark_trace_histo *histo, enum kshark_config_formats format) { - /* Create a new Configuration document. */ - struct kshark_config_doc *conf = - kshark_config_new("kshark.config.model", format); - - if (!conf) - return NULL; + struct kshark_config_doc *conf;switch (format) {case KS_CONFIG_JSON: - kshark_model_to_json(histo, conf->conf_doc); - return conf; + break;default:fprintf(stderr, "Document format %d not supported\n", format); return NULL; } + + /* Create a new Configuration document. */ + conf = kshark_config_new("kshark.config.model", format); + if (!conf) + return NULL; + + kshark_model_to_json(histo, conf->conf_doc); + return conf; }static bool kshark_model_from_json(struct kshark_trace_histo *histo,
|