Re: [PATCH v2 bpf-next 07/11] libbpf: add BPF static linker BTF and BTF.ext support

On Tue, Mar 16, 2021 at 10:25 PM Alexei Starovoitov
[off-list ref] wrote:

On Sat, Mar 13, 2021 at 11:35:33AM -0800, Andrii Nakryiko wrote:

quoted

+             for (j = 0; j < n; j++, src_var++) {
+                     void *sec_vars = dst_sec->sec_vars;
+
+                     sec_vars = libbpf_reallocarray(sec_vars,
+                                                    dst_sec->sec_var_cnt + 1,
+                                                    sizeof(*dst_sec->sec_vars));
+                     if (!sec_vars)
+                             return -ENOMEM;
+
+                     dst_sec->sec_vars = sec_vars;
+                     dst_sec->sec_var_cnt++;
+
+                     dst_var = &dst_sec->sec_vars[dst_sec->sec_var_cnt - 1];
+                     dst_var->type = obj->btf_type_map[src_var->type];
+                     dst_var->size = src_var->size;
+                     dst_var->offset = src_sec->dst_off + src_var->offset;
+             }
+     }
+
+     return 0;
+}
+
+static void *add_btf_ext_rec(struct btf_ext_sec_data *ext_data, const void *src_rec)
+{
+     size_t new_sz = (ext_data->rec_cnt + 1) * ext_data->rec_sz;
+     void *tmp;
+
+     tmp = realloc(ext_data->recs, new_sz);
+     if (!tmp)
+             return NULL;
+
+     ext_data->recs = tmp;
+     ext_data->rec_cnt++;
+
+     tmp += new_sz - ext_data->rec_sz;
+     memcpy(tmp, src_rec, ext_data->rec_sz);

while reading this and previous patch the cnt vs sz difference was
constantly throwing me off. Not a big deal, of course.
Did you consider using _cnt everywhere and use finalize method
to convert everything to size?
Like in this function libbpf_reallocarray() instead of realloc() would
probably be easier to read and more consistent, since btf_ext_sec_data
is measuring things in _cnt.

will switch this and add_sym() to reallocarray, given both are dealing
with real fixed-size records (not just bytes)

In the previous patch the section is in _sz which I guess is necessary
because sections can contain differently sized objects?

yes, it could be records of different sizes (e.g., relocations,
symbols), or just unstructured data (e.g., .data, string table, etc).

btw, strset abstraction is really nice. It made the patches much easier
to read.

Thanks. Yeah, it simplified existing BTF/btf_dedup logic quite a bit
as well, it was a good suggestion!