On Thu, Apr 22, 2021 at 11:24 AM Andrii Nakryiko <andrii.nakryiko@xxxxxxxxx> wrote: > > On Thu, Apr 22, 2021 at 9:50 AM Yonghong Song <yhs@xxxxxx> wrote: > > > > > > > > On 4/16/21 1:23 PM, Andrii Nakryiko wrote: > > > It should never fail, but if it does, it's better to know about this rather > > > than end up with nonsensical type IDs. > > > > So this is defensive programming. Maybe do another round of > > audit of the callers and if you didn't find any issue, you > > do not need to check not-happening condition here? > > It's far from obvious that this will never happen, because we do a > decently complicated BTF processing (we skip some types altogether > believing that they are not used, for example) and it will only get > more complicated with time. Just as there are "verifier bug" checks in > kernel, this prevents things from going wild if non-trivial bugs will > inevitably happen. I agree with Yonghong. This doesn't look right. The callback will be called for all non-void types, right? so *type_id == 0 shouldn't never happen. If it does there is a bug somewhere that should be investigated instead of ignored. The if (new_id == 0) pr_warn bit makes sense. My reading that it will abort the whole linking process and this linker bug will be reported back to us. So it's good.
