On Mon, Sep 10, 2012 at 03:53:51PM +0800, Yan, Zheng wrote:
> From: "Yan, Zheng" <zheng.z....@intel.com>
SNIP
> +{
> + struct perf_evsel *evsel;
>
> + while (!list_empty(list)) {
> + evsel = list_entry(list->next, struct perf_evsel, node);
> + list_del(&evsel->node);
> + perf_evsel__delete(evsel);
> + }
> + free(list);
> +}
>
> -static int __add_event(struct list_head **_list, int *idx,
> - struct perf_event_attr *attr,
> - char *name, struct cpu_map *cpus)
> +static struct perf_evsel *__add_event(int *idx, struct perf_event_attr *attr,
> + char *name, struct cpu_map *cpus)
> {
> struct perf_evsel *evsel;
> +
> + event_attr_init(attr);
> +
> + evsel = perf_evsel__new(attr, (*idx)++);
> + if (!evsel)
> + return NULL;
> +
> + evsel->cpus = cpus;
> + if (name)
> + evsel->name = strdup(name);
> + return evsel;
> +}
> +
> +static int add_event(struct list_head **_list, int *idx,
> + struct perf_event_attr *attr, char *name)
> +{
> struct list_head *list = *_list;
> + struct perf_evsel *evsel;
>
> if (!list) {
> list = malloc(sizeof(*list));
> @@ -255,28 +281,17 @@ static int __add_event(struct list_head **_list, int
> *idx,
> INIT_LIST_HEAD(list);
> }
>
> - event_attr_init(attr);
> -
> - evsel = perf_evsel__new(attr, (*idx)++);
> + evsel = __add_event(idx, attr, name, NULL);
> if (!evsel) {
> free(list);
> return -ENOMEM;
> }
>
> - evsel->cpus = cpus;
> - if (name)
> - evsel->name = strdup(name);
> list_add_tail(&evsel->node, list);
> *_list = list;
> return 0;
> }
>
> -static int add_event(struct list_head **_list, int *idx,
> - struct perf_event_attr *attr, char *name)
> -{
> - return __add_event(_list, idx, attr, name, NULL);
> -}
> -
Would it be possible to have all '*add_event' more obvious for usage.
Also following code is duplicated after each call of __add_event:
evsel = __add_event(idx, &attr, pmu_event_name(head_config),
pmu->cpus);
if (!evsel) {
*idx = orig_idx;
free_event_list(list);
return -ENOMEM;
}
list_add_tail(&evsel->node, list);
I tried some changes over your patch.. just compiled, not tested ;)
It should also solve the issue from my other comment.
thanks,
jirka
---
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 1b0a46f..f6bbd7a 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -268,8 +268,10 @@ static struct perf_evsel *__add_event(int *idx, struct
perf_event_attr *attr,
return evsel;
}
-static int add_event(struct list_head **_list, int *idx,
- struct perf_event_attr *attr, char *name)
+static struct perf_evsel*
+add_event_list(struct list_head **_list, int *idx,
+ struct perf_event_attr *attr, char *name,
+ struct cpu_map *cpus)
{
struct list_head *list = *_list;
struct perf_evsel *evsel;
@@ -277,19 +279,27 @@ static int add_event(struct list_head **_list, int *idx,
if (!list) {
list = malloc(sizeof(*list));
if (!list)
- return -ENOMEM;
+ return NULL;
INIT_LIST_HEAD(list);
}
- evsel = __add_event(idx, attr, name, NULL);
+ evsel = __add_event(idx, attr, name, cpus);
if (!evsel) {
- free(list);
- return -ENOMEM;
+ free_event_list(list);
+ return NULL;
}
list_add_tail(&evsel->node, list);
- *_list = list;
- return 0;
+ if (!*_list)
+ *_list = list;
+
+ return evsel;
+}
+
+static int add_event(struct list_head **_list, int *idx,
+ struct perf_event_attr *attr, char *name)
+{
+ return add_event_list(_list, idx, attr, name, NULL) ? 0 : -ENOMEM;
}
static int parse_aliases(char *str, const char
*names[][PERF_EVSEL__MAX_ALIASES], int size)
@@ -635,16 +645,10 @@ int parse_events_add_pmu(struct list_head **_list, int
*idx,
char *name, struct list_head *head_config)
{
struct perf_event_attr attr;
- struct list_head *list;
struct perf_pmu *pmu = NULL;
struct perf_evsel *evsel, *first = NULL;
int orig_idx = *idx;
- list = malloc(sizeof(*list));
- if (!list)
- return -ENOMEM;
- INIT_LIST_HEAD(list);
-
while ((pmu = perf_pmu__scan(pmu))) {
if (pmu_name_match(pmu->name, name))
continue;
@@ -663,14 +667,12 @@ int parse_events_add_pmu(struct list_head **_list, int
*idx,
if (perf_pmu__config(pmu, &attr, head_config))
return -EINVAL;
- evsel = __add_event(idx, &attr, pmu_event_name(head_config),
- pmu->cpus);
+ evsel = add_event_list(_list, idx, &attr,
pmu_event_name(head_config),
+ pmu->cpus);
if (!evsel) {
*idx = orig_idx;
- free_event_list(list);
return -ENOMEM;
}
- list_add_tail(&evsel->node, list);
if (first) {
list_add_tail(&evsel->sibling, &first->sibling);
evsel->aggr_slave = true;
@@ -679,7 +681,6 @@ int parse_events_add_pmu(struct list_head **_list, int *idx,
}
}
- *_list = list;
return 0;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
