[DTrace-devel] [PATCH] cg: rework data size to BPF size conversion and improve error reporting

[Kris Van Hees]

On Tue, Sep 05, 2023 at 06:58:21PM -0400, Eugene Loh via DTrace-devel wrote:
> Reviewed-by: Eugene Loh <eugene.loh at oracle.com>
> 
> I don't know in what order patches will appear, but there might still be a
> vestigial assert() in the ip provider to be removed.

This will go in before the IP provider.

> Also, in:
> 
> +/*
> + * Variant of store and load macros that takes an actual data size and call
> + * ldstsz(sz) to determine the instruction size specifier.
> + */
> +#define BPF_STOREX(sz, dst, ofs, src)                                  \
> +               BPF_STORE(bpf_ldst_size(sz, 1), (dst), (ofs), (src))
> +#define BPF_STOREX_IMM(sz, dst, ofs, src)                              \
> +               BPF_STORE_IMM(bpf_ldst_size(sz, 1), (dst), (ofs), (src))
> +#define BPF_LOADX(sz, dst, src, ofs)                                   \
> +               BPF_LOAD(bpf_ldst_size(sz, 0), (dst), (src), (ofs))
> +
> 
> maybe that comment about ldstsz(sz) should be bpf_ldst_size(). Also,

Yes.

> grammatically, the comment should be consistent about singular/plural. 
> Either "variant takes calls" or "take call". Maybe s/Variant/Variants/ and
> s/takes/take/.

I don't think we worry that much about perfect grammar, but since I am updating
the comment anyway, I'll fix it.