On Thu, Mar 25, 2021 at 06:33:14PM +0800, John Garry wrote:
SNIP
+struct metric {
+ struct list_head list;
+ struct metric_ref metric_ref;
+};
+
+static int resolve_metric_simple(struct expr_parse_ctx *pctx,
+ struct list_head *compound_list,
+ struct pmu_events_map *map,
+ const char *metric_name)
+{
+ struct hashmap_entry *cur, *cur_tmp;
+ struct metric *metric, *tmp;
+ size_t bkt;
+ bool all;
+ int rc;
+
+ do {
+ all = true;
+ hashmap__for_each_entry_safe((&pctx->ids), cur, cur_tmp, bkt) {
+ struct metric_ref *ref;
+ struct pmu_event *pe;
+
+ pe = metrcgroup_find_metric(cur->key, map);
+ if (!pe)
+ continue;
+
+ if (!strcmp(metric_name, (char *)cur->key)) {
+ pr_warning("Recursion detected for metric %s\n", metric_name);
+ rc = -1;
+ goto out_err;
+ }
+
+ all = false;
+
+ /* The metric key itself needs to go out.. */
+ expr__del_id(pctx, cur->key);
+
+ metric = malloc(sizeof(*metric));
+ if (!metric) {
+ rc = -ENOMEM;
+ goto out_err;
+ }
+
+ ref = &metric->metric_ref;
+ ref->metric_name = pe->metric_name;
+ ref->metric_expr = pe->metric_expr;
+ list_add_tail(&metric->list, compound_list);
+
+ rc = expr__find_other(pe->metric_expr, NULL, pctx, 0);
so this might add new items to pctx->ids, I think you need
to restart the iteration as we do it in __resolve_metric
otherwise you could miss some new keys
jirka
+ if (rc)
+ goto out_err;
+ }
+ } while (!all);
+
+ return 0;
+
+out_err:
+ list_for_each_entry_safe(metric, tmp, compound_list, list)
+ free(metric);
+
+ return rc;
+
+}
SNIP
.