On Mon, Oct 11, 2021 at 9:15 PM Kumar Kartikeya Dwivedi
<memxor@xxxxxxxxx> wrote:
>
> On Fri, Oct 08, 2021 at 05:32:59AM IST, andrii.nakryiko@xxxxxxxxx wrote:
> > From: Andrii Nakryiko <andrii@xxxxxxxxxx>
> >
> > [...]
> > One interesting possibility that is now open by these changes is that it's
> > possible to do:
> >
> > bpf_trace_printk("My fmt %s", sizeof("My fmt %s"), "blah");
> >
> > and it will work as expected. I haven't updated libbpf-provided helpers in
> > bpf_helpers.h for snprintf, seq_printf, and printk, because using
> > `static const char ___fmt[] = fmt;` trick is still efficient and doesn't fill
> > out the buffer at runtime (no copying), but it also enforces that format
> > string is compile-time string literal:
> >
> > const char *s = NULL;
> >
> > bpf_printk("hi"); /* works */
> > bpf_printk(s); /* compilation error */
> >
> > By passing fmt directly to bpf_trace_printk() would actually compile
> > bpf_printk(s) above with no warnings and will not work at runtime, which is
> > worse user experience, IMO.
> >
>
> You could try the following (_Static_assert would probably be preferable, but
> IDK if libbpf can use it).
Yeah, we definitely can use _Static_assert from BPF side (see
progs/test_cls_redirect.c in selftests/bpf).
>
> #define IS_ARRAY(x) ({ sizeof(int[-__builtin_types_compatible_p(typeof(x), typeof(&*(x)))]); 1; })
>
Thanks! This seems to be working well to detect arrays and string
literals. I'll keep it in my toolbox :) Ultimately I decided to not
touch bpf_printk() for now, because of the BPF_NO_GLOBAL_DATA trick.
If I go with direct "fmt" usage, I'll need to implementations of
__bpf_printk(), which doesn't seem worth it right now. I might revisit
it later, though.
> In action: https://godbolt.org/z/4d4W61YPf
>
> --
> Kartikeya
