Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1417361 > unrolled thread

[PATCH v8 4/5] perf config: Use zfree() instead of free() at perf_config_set__delete()

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v8 4/5] perf config: Use zfree() instead of free() at perf_config_set__delete() Taeung Song <treeze.taeung@gmail.com> - 2016-06-08 14:40 +0200
    Re: [PATCH v8 4/5] perf config: Use zfree() instead of free() at  perf_config_set__delete() Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-06-09 15:40 +0200
      Re: [PATCH v8 4/5] perf config: Use zfree() instead of free() at  perf_config_set__delete() Taeung Song <treeze.taeung@gmail.com> - 2016-06-10 13:10 +0200

#1417361 — [PATCH v8 4/5] perf config: Use zfree() instead of free() at perf_config_set__delete()

perf_config_set__delete() delete allocated the config set
but the global variable 'config_set' is used all around.

So purge and zfree by an address of the global variable
, i.e. 'struct perf_config_set **' type
instead of using local variable 'set' of which type
is 'struct perf_config_set *'.

Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Wang Nan <wangnan0@huawei.com>
Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
---
 tools/perf/builtin-config.c |  2 +-
 tools/perf/perf.c           |  2 +-
 tools/perf/util/config.c    | 10 +++++-----
 tools/perf/util/config.h    |  2 +-
 4 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/tools/perf/builtin-config.c b/tools/perf/builtin-config.c
index cfd1036..c07f744 100644
--- a/tools/perf/builtin-config.c
+++ b/tools/perf/builtin-config.c
@@ -110,7 +110,7 @@ int cmd_config(int argc, const char **argv, const char *prefix __maybe_unused)
 		usage_with_options(config_usage, config_options);
 	}
 
-	perf_config_set__delete(set);
+	perf_config_set__delete(&set);
 out_err:
 	return ret;
 }
diff --git a/tools/perf/perf.c b/tools/perf/perf.c
index fe2ab7c..058d5dc 100644
--- a/tools/perf/perf.c
+++ b/tools/perf/perf.c
@@ -391,7 +391,7 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv)
 
 	perf_env__set_cmdline(&perf_env, argc, argv);
 	status = p->fn(argc, argv, prefix);
-	perf_config_set__delete(config_set);
+	perf_config_set__delete(&config_set);
 	exit_browser(status);
 	perf_env__exit(&perf_env);
 	bpf__clear();
diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
index 72db134..23fb8e4 100644
--- a/tools/perf/util/config.c
+++ b/tools/perf/util/config.c
@@ -654,7 +654,7 @@ struct perf_config_set *perf_config_set__new(void)
 	if (set) {
 		INIT_LIST_HEAD(&set->sections);
 		if (perf_config_set__init(set) < 0) {
-			perf_config_set__delete(set);
+			perf_config_set__delete(&set);
 			set = NULL;
 		}
 	}
@@ -737,13 +737,13 @@ static void perf_config_set__purge(struct perf_config_set *set)
 	}
 }
 
-void perf_config_set__delete(struct perf_config_set *set)
+void perf_config_set__delete(struct perf_config_set **set)
 {
-	if (set == NULL)
+	if (*set == NULL)
 		return;
 
-	perf_config_set__purge(set);
-	free(set);
+	perf_config_set__purge(*set);
+	zfree(set);
 }
 
 /*
diff --git a/tools/perf/util/config.h b/tools/perf/util/config.h
index 7cc4fea..fafba86 100644
--- a/tools/perf/util/config.h
+++ b/tools/perf/util/config.h
@@ -34,6 +34,6 @@ const char *perf_config_dirname(const char *, const char *);
 const char *perf_etc_perfconfig(void);
 
 struct perf_config_set *perf_config_set__new(void);
-void perf_config_set__delete(struct perf_config_set *set);
+void perf_config_set__delete(struct perf_config_set **set);
 
 #endif /* __PERF_CONFIG_H */
-- 
2.5.0

[toc] | [next] | [standalone]

#1418325 — Re: [PATCH v8 4/5] perf config: Use zfree() instead of free() at perf_config_set__delete()

Em Wed, Jun 08, 2016 at 09:36:52PM +0900, Taeung Song escreveu:
> perf_config_set__delete() delete allocated the config set
> but the global variable 'config_set' is used all around.
 
> So purge and zfree by an address of the global variable
> , i.e. 'struct perf_config_set **' type
> instead of using local variable 'set' of which type
> is 'struct perf_config_set *'.

> -void perf_config_set__delete(struct perf_config_set *set)
> +void perf_config_set__delete(struct perf_config_set **set)
>  {
> -	if (set == NULL)
> +	if (*set == NULL)
>  		return;
>  
> -	perf_config_set__purge(set);
> -	free(set);
> +	perf_config_set__purge(*set);
> +	zfree(set);
>  }

Nope, don't change conventions like taht, a delete method should not
receive a pointer to the pointer to be deleted, no odd cases, please.

If you really think this is interesting, please introduce zdelete(),
i.e.:

void perf_config_set__zdelete(struct perf_config_set **set)
{
	if (!set)
		return;

	perf_config_set__delete(*set);
	*set = NULL;
}

- Arnaldo

[toc] | [prev] | [next] | [standalone]

#1419268 — Re: [PATCH v8 4/5] perf config: Use zfree() instead of free() at perf_config_set__delete()

On 06/09/2016 10:37 PM, Arnaldo Carvalho de Melo wrote:
> Em Wed, Jun 08, 2016 at 09:36:52PM +0900, Taeung Song escreveu:
>> perf_config_set__delete() delete allocated the config set
>> but the global variable 'config_set' is used all around.
>
>> So purge and zfree by an address of the global variable
>> , i.e. 'struct perf_config_set **' type
>> instead of using local variable 'set' of which type
>> is 'struct perf_config_set *'.
>
>> -void perf_config_set__delete(struct perf_config_set *set)
>> +void perf_config_set__delete(struct perf_config_set **set)
>>   {
>> -	if (set == NULL)
>> +	if (*set == NULL)
>>   		return;
>>
>> -	perf_config_set__purge(set);
>> -	free(set);
>> +	perf_config_set__purge(*set);
>> +	zfree(set);
>>   }
>
> Nope, don't change conventions like taht, a delete method should not
> receive a pointer to the pointer to be deleted, no odd cases, please.
>
> If you really think this is interesting, please introduce zdelete(),
> i.e.:
>
> void perf_config_set__zdelete(struct perf_config_set **set)
> {
> 	if (!set)
> 		return;
>
> 	perf_config_set__delete(*set);
> 	*set = NULL;
> }
>

I understood!

If we don't use the config set as a global variable,
it seems that we wouldn't need to change perf_config_set__delete() to 
perf_config_set__zdelete() as this patch.

Thanks,
Taeung

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web