Messages in this thread | | | Subject | Re: [PATCH v2 2/6] perf annotate: Simplify disasm_line allocation and freeing code | From | Ravi Bangoria <> | Date | Tue, 28 Jan 2020 19:27:29 +0530 |
| |
Hi Jiri,
On 1/27/20 5:29 PM, Jiri Olsa wrote: > 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
Sorry, I didn't get your point about 'specialized' line. You mean the way I'm doing is better wrt existing code?
> >> - 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
dl->al.line is allocated using strdup() in annotation_line__init(). So I think we still need that zfree(&dl->al.line).
> also could you please put it to annotation_line__exit, since > you already added the __init function
Sure will do it.
Thanks for review :) Ravi
| |