On 04/01/2016 11:35 PM, Arnaldo Carvalho de Melo wrote:
Em Fri, Apr 01, 2016 at 07:27:22PM +0900, Taeung Song escreveu:
On 04/01/2016 02:31 AM, Arnaldo Carvalho de Melo wrote:
Em Tue, Mar 29, 2016 at 09:43:13AM +0900, Taeung Song escreveu:
+static int collect_config(const char *var, const char *value,
+                         void *perf_config_set)
+{
+       int ret = -1;
+       char *ptr, *key;
+       char *section_name, *name;
+       struct perf_config_section *section = NULL;
+       struct perf_config_item *config_item = NULL;
+       struct perf_config_set *perf_configs = perf_config_set;
+       struct list_head *sections = &perf_configs->sections;

+       key = ptr = strdup(var);
+       if (!key) {
+               pr_err("%s: strdup failed\n", __func__);

pr_debug()

I'll change pr_err to pr_debug.
But why do use pr_debug at only this part ?

Well, ideally one would propagate the errors from library level code to
the code in the tool, i.e. closer to builtin-foo.c, where it would
decide how to present it to the user.

For extra messages, that may help a more advanced user or to be sent to
the tool developers, use pr_debug().

I understood it!


+struct perf_config_section {
+       char *name;
+       struct list_head config_items;

s/config_items/items/g

+       struct list_head list;

s/list/node/g
+};
+
+struct perf_config_set {
+       struct list_head sections;

See? Here you did it right, no point in having it as "config_sections"

I'll rename it to 'config_sections'.

I meant to keep this one like it is, i.e. perf_config_set->sections, and
use the same principle: shorter names, removing useless stuff like
"config_", that in this case can be implied by the name of the class,
and apply it to the perf_config_section case, making it
perf_config_section->items, ok?

I got it. :-)

I'll resend v5

Thanks,
Taeung

Reply via email to