On Wed, 2024-10-09 at 23:35 +0000, Ihor Solodrai wrote:
> Extract elf_functions struct type from btf_encoder.
>
> Rewrite btf_encoder__collect_function() into
> elf_functions__collect_function(), and btf_encoder__collect_symbols()
> into elf_functions__collect().
Nit:
the sentence above is somewhat cryptic. I think it's better to say
something like:
Extract struct elf_functions 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>
> ---
Modulo a few nitpicks this patch looks ok to me.
Reviewed-by: Eduard Zingerman <eddyz87@xxxxxxxxx>
[...]
> @@ -2100,22 +2075,38 @@ int btf_encoder__encode(struct btf_encoder *encoder)
> }
>
>
> -static int btf_encoder__collect_symbols(struct btf_encoder *encoder)
> +static int elf_functions__collect(struct elf_functions *functions)
> {
> - uint32_t sym_sec_idx;
> + uint32_t nr_symbols = elf_symtab__nr_symbols(functions->symtab);
> + Elf32_Word sym_sec_idx;
> uint32_t core_id;
> GElf_Sym sym;
>
> - elf_symtab__for_each_symbol_index(encoder->symtab, core_id, sym, sym_sec_idx) {
> - if (btf_encoder__collect_function(encoder, &sym))
> + // 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));
> + if (!functions->entries) {
> + fprintf(stderr, "could not allocate memory for elf_functions table\n");
> + return -ENOMEM;
> + }
> +
> + functions->cnt = 0;
> + elf_symtab__for_each_symbol_index(functions->symtab, core_id, sym, sym_sec_idx) {;
> + if (elf_functions__collect_function(functions, &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);
> +
> + // Reallocate to the exact size
> + functions->entries = realloc(functions->entries, functions->cnt * sizeof(struct elf_function));
You need to store realloc result to a temporary, otherwise pointer to
previously allocated memory in functions->entries is lost.
(Although idk if such realloc could fail).
> + if (!functions->entries) {
> + fprintf(stderr, "could not reallocate memory for elf_functions table\n");
> + return -ENOMEM;
> }
>
> return 0;
[...]
