On Thu, 2024-11-28 at 01:24 +0000, Ihor Solodrai wrote:
> Extract elf_functions struct type from btf_encoder.
>
> Replace methods operating functions table in btf_encoder by methods
> operating on elf_functions:
> - btf_encoder__collect_function -> elf_functions__collect_function
> - btf_encoder__collect_symbols -> elf_functions__collect
>
> Now these functions do not depend on btf_encoder being passed to them
> as a parameter.
>
> Signed-off-by: Ihor Solodrai <ihor.solodrai@xxxxx>
> ---
Acked-by: Eduard Zingerman <eddyz87@xxxxxxxxx>
[...]
> @@ -2132,31 +2110,59 @@ int btf_encoder__encode(struct btf_encoder *encoder)
> return err;
> }
>
> -
> -static int btf_encoder__collect_symbols(struct btf_encoder *encoder)
> +static int elf_functions__collect(struct elf_functions *functions)
> {
> - bool base_addr_set = false;
> - uint32_t sym_sec_idx;
> + uint32_t nr_symbols = elf_symtab__nr_symbols(functions->symtab);
> + struct elf_function *tmp;
> + Elf32_Word sym_sec_idx;
> uint32_t core_id;
> GElf_Sym sym;
> + int err;
> +
> + /* We know that number of functions is less than number of symbols,
> + * so we can overallocate temporarily.
> + */
> + functions->entries = calloc(nr_symbols, sizeof(struct elf_function));
Nit: use sizeof(*functions->entries) instead of sizeof(struct elf_function)
here and elsewhere.
> + if (!functions->entries) {
> + err = -ENOMEM;
> + goto out_free;
> + }
>
> - elf_symtab__for_each_symbol_index(encoder->symtab, core_id, sym, sym_sec_idx) {
> - if (!base_addr_set && sym_sec_idx && sym_sec_idx < encoder->seccnt) {
> - encoder->functions.base_addr = encoder->secinfo[sym_sec_idx].addr;
> - base_addr_set = true;
> + functions->cnt = 0;
> + elf_symtab__for_each_symbol_index(functions->symtab, core_id, sym, sym_sec_idx) {
> + if (elf_functions__collect_function(functions, &sym)) {
> + err = -1;
> + goto out_free;
Nit: elf_functions__collect_function() never fails now (make it void?).
> }
> - if (btf_encoder__collect_function(encoder, &sym))
> - return -1;
> }
>
> - if (encoder->functions.cnt) {
> - qsort(encoder->functions.entries, encoder->functions.cnt, sizeof(encoder->functions.entries[0]),
> + if (functions->cnt) {
> + qsort(functions->entries,
> + functions->cnt,
> + sizeof(functions->entries[0]),
> functions_cmp);
> - if (encoder->verbose)
> - printf("Found %d functions!\n", encoder->functions.cnt);
> + } else {
> + err = 0;
> + goto out_free;
> + }
> +
> + /* Reallocate to the exact size */
> + tmp = realloc(functions->entries, functions->cnt * sizeof(struct elf_function));
> + if (tmp) {
> + functions->entries = tmp;
> + } else {
> + fprintf(stderr, "could not reallocate memory for elf_functions table\n");
> + err = -ENOMEM;
> + goto out_free;
> }
>
> return 0;
> +
> +out_free:
> + free(functions->entries);
> + functions->entries = NULL;
> + functions->cnt = 0;
> + return err;
> }
>
> static bool ftype__has_arg_names(const struct ftype *ftype)
> @@ -2417,6 +2423,7 @@ struct btf_encoder *btf_encoder__new(struct cu *cu, const char *detached_filenam
> printf("%s: '%s' doesn't have symtab.\n", __func__, cu->filename);
> goto out;
> }
> + encoder->functions.symtab = encoder->symtab;
>
> /* index the ELF sections for later lookup */
>
> @@ -2455,7 +2462,7 @@ struct btf_encoder *btf_encoder__new(struct cu *cu, const char *detached_filenam
> if (!found_percpu && encoder->verbose)
> printf("%s: '%s' doesn't have '%s' section\n", __func__, cu->filename, PERCPU_SECTION);
>
> - if (btf_encoder__collect_symbols(encoder))
> + if (elf_functions__collect(&encoder->functions))
> goto out_delete;
>
> if (encoder->verbose)
> @@ -2486,7 +2493,7 @@ void btf_encoder__delete(struct btf_encoder *encoder)
> encoder->btf = NULL;
> elf_symtab__delete(encoder->symtab);
>
> - encoder->functions.allocated = encoder->functions.cnt = 0;
> + encoder->functions.cnt = 0;
> free(encoder->functions.entries);
> encoder->functions.entries = NULL;
Nit: this cleanup code is repeated two times.
