diff mbox series

[20/34] kernelshark: Fix potential memory leaks in libkshark-configio

Message ID 20240114171723.14092-21-dev@benjarobin.fr (mailing list archive)
State New
Headers show
Series Fix kernelshark issues introduced by the migration to Qt6 | expand

Commit Message

Benjamin ROBIN Jan. 14, 2024, 5:17 p.m. UTC 
Allocate a new kshark_config_doc only if the format is supported.

Signed-off-by: Benjamin ROBIN <dev@benjarobin.fr>
---
 src/libkshark-configio.c | 74 ++++++++++++++++++++++------------------
 1 file changed, 41 insertions(+), 33 deletions(-)

Comments

Yordan Karadzhov Jan. 21, 2024, 6:41 p.m. UTC | #1 
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@benjarobin.fr>
> ---
>   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,
diff mbox series

Patch

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;
+		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;
+		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,