Messages in this thread | | | Date | Mon, 27 Jan 2020 12:59:11 +0100 | From | Jiri Olsa <> | Subject | Re: [PATCH v2 2/6] perf annotate: Simplify disasm_line allocation and freeing code |
| |
On Fri, Jan 24, 2020 at 01:34:28PM +0530, Ravi Bangoria wrote:
SNIP
> > /* > * Allocating the disasm annotation line data with > * following structure: > * > - * ------------------------------------------------------------ > - * privsize space | struct disasm_line | struct annotation_line > - * ------------------------------------------------------------ > + * ------------------------------------------- > + * struct disasm_line | struct annotation_line > + * ------------------------------------------- > * > * We have 'struct annotation_line' member as last member > * of 'struct disasm_line' to have an easy access. > - * > */ > static struct disasm_line *disasm_line__new(struct annotate_args *args) > { > struct disasm_line *dl = NULL; > - struct annotation_line *al; > - size_t privsize = args->privsize + offsetof(struct disasm_line, al); > + int nr = 1; > > - al = annotation_line__new(args, privsize);
ok, I finally recalled why we did it like this.. for the python annotation support, which never made it in ;-) however the allocation in 'specialized' line and later call to annotation_line__init might actualy be a better way
> - if (al != NULL) { > - dl = disasm_line(al); > + if (perf_evsel__is_group_event(args->evsel)) > + nr = args->evsel->core.nr_members; > > - if (dl->al.line == NULL) > - goto out_delete; > + dl = zalloc(disasm_line_size(nr)); > + if (!dl) > + return NULL; > > - if (args->offset != -1) { > - if (disasm_line__parse(dl->al.line, &dl->ins.name, &dl->ops.raw) < 0) > - goto out_free_line; > + annotation_line__init(&dl->al, args, nr); > + if (dl->al.line == NULL) > + goto out_delete; > > - disasm_line__init_ins(dl, args->arch, &args->ms); > - } > + if (args->offset != -1) { > + if (disasm_line__parse(dl->al.line, &dl->ins.name, &dl->ops.raw) < 0) > + goto out_free_line; > + > + disasm_line__init_ins(dl, args->arch, &args->ms); > } > > return dl; > @@ -1248,7 +1219,9 @@ void disasm_line__free(struct disasm_line *dl) > else > ins__delete(&dl->ops); > zfree(&dl->ins.name); > - annotation_line__delete(&dl->al); > + free_srcline(dl->al.path); > + zfree(&dl->al.line);
no need to zfree if you're freeing the memory on the next line also could you please put it to annotation_line__exit, since you already added the __init function
> + free(dl); > } >
the rest of the patches look good to me
thanks, jirka
| |