> On Jul 30, 2019, at 11:52 PM, Andrii Nakryiko <andrii.nakryiko@xxxxxxxxx> wrote: > > On Tue, Jul 30, 2019 at 10:19 PM Song Liu <songliubraving@xxxxxx> wrote: >> >> >> >>> On Jul 30, 2019, at 6:00 PM, Andrii Nakryiko <andrii.nakryiko@xxxxxxxxx> wrote: >>> >>> On Tue, Jul 30, 2019 at 5:39 PM Song Liu <songliubraving@xxxxxx> wrote: >>>> >>>> >>>> >>>>> On Jul 30, 2019, at 12:53 PM, Andrii Nakryiko <andriin@xxxxxx> wrote: >>>>> >>>>> This patch implements the core logic for BPF CO-RE offsets relocations. >>>>> Every instruction that needs to be relocated has corresponding >>>>> bpf_offset_reloc as part of BTF.ext. Relocations are performed by trying >>>>> to match recorded "local" relocation spec against potentially many >>>>> compatible "target" types, creating corresponding spec. Details of the >>>>> algorithm are noted in corresponding comments in the code. >>>>> >>>>> Signed-off-by: Andrii Nakryiko <andriin@xxxxxx> [...] >>>> >>> >>> I just picked the most succinct and non-repetitive form. It's >>> immediately apparent which type it's implicitly converted to, so I >>> felt there is no need to repeat it. Also, just (void *) is much >>> shorter. :) >> >> _All_ other code in btf.c converts the pointer to the target type. > > Most in libbpf.c doesn't, though. Also, I try to preserve pointer > constness for uses that don't modify BTF types (pretty much all of > them in libbpf), so it becomes really verbose, despite extremely short > variable names: > > const struct btf_member *m = (const struct btf_member *)(t + 1); I don't think being verbose is a big problem here. Overusing (void *) feels like a bigger problem. > > Add one or two levels of nestedness and you are wrapping this line. > >> In some cases, it is not apparent which type it is converted to, >> for example: >> >> + m = (void *)(targ_type + 1); >> >> I would suggest we do implicit conversion whenever possible. > > Implicit conversion (`m = targ_type + 1;`) is a compilation error, > that won't work. I misused "implicit" here. I actually meant to say m = ((const struct btf_member *)(t + 1);
