On Jun 10 2024, Alexei Starovoitov wrote:
> On Sat, Jun 8, 2024 at 2:01 AM Benjamin Tissoires <bentiss@xxxxxxxxxx> wrote:
> >
> > +
> > +static int hid_bpf_ops_init_member(const struct btf_type *t,
> > + const struct btf_member *member,
> > + void *kdata, const void *udata)
> > +{
> > + u32 moff = __btf_member_bit_offset(t, member) / 8;
> > + u32 flags;
> > +
> > + switch (moff) {
> > + case offsetof(struct hid_bpf_ops, hid_id):
> > + /* For hid_id and flags fields, this function has to copy it
> > + * and return 1 to indicate that the data has been handled by
> > + * the struct_ops type, or the verifier will reject the map if
> > + * the value of those fields is not zero.
> > + */
> > + ((struct hid_bpf_ops *)kdata)->hid_id = ((struct hid_bpf_ops *)udata)->hid_id;
> > + return 1;
> > + case offsetof(struct hid_bpf_ops, flags):
> > + flags = ((struct hid_bpf_ops *)udata)->flags;
> > + if (flags & ~BPF_F_BEFORE)
> > + return -EINVAL;
> > + ((struct hid_bpf_ops *)kdata)->flags = flags;
>
> minor nit: I'd cast kdata/udate in the beginning of
> the function to make the lines shorter and less verbose.
> Similar to how bpf_tcp_ca_init_member() does it.
The change looks like:
---
diff --git a/drivers/hid/bpf/hid_bpf_struct_ops.c b/drivers/hid/bpf/hid_bpf_struct_ops.c
index 9192c66cde20..dde547e734ed 100644
--- a/drivers/hid/bpf/hid_bpf_struct_ops.c
+++ b/drivers/hid/bpf/hid_bpf_struct_ops.c
@@ -97,8 +97,14 @@ static int hid_bpf_ops_init_member(const struct btf_type *t,
const struct btf_member *member,
void *kdata, const void *udata)
{
- u32 moff = __btf_member_bit_offset(t, member) / 8;
- u32 flags;
+ const struct hid_bpf_ops *uhid_bpf_ops;
+ struct hid_bpf_ops *khid_bpf_ops;
+ u32 moff;
+
+ uhid_bpf_ops = (const struct hid_bpf_ops *)udata;
+ khid_bpf_ops = (struct hid_bpf_ops *)kdata;
+
+ moff = __btf_member_bit_offset(t, member) / 8;
switch (moff) {
case offsetof(struct hid_bpf_ops, hid_id):
@@ -107,13 +113,12 @@ static int hid_bpf_ops_init_member(const struct btf_type *t,
* the struct_ops type, or the verifier will reject the map if
* the value of those fields is not zero.
*/
- ((struct hid_bpf_ops *)kdata)->hid_id = ((struct hid_bpf_ops *)udata)->hid_id;
+ khid_bpf_ops->hid_id = uhid_bpf_ops->hid_id;
return 1;
case offsetof(struct hid_bpf_ops, flags):
- flags = ((struct hid_bpf_ops *)udata)->flags;
- if (flags & ~BPF_F_BEFORE)
+ if (uhid_bpf_ops->flags & ~BPF_F_BEFORE)
return -EINVAL;
- ((struct hid_bpf_ops *)kdata)->flags = flags;
+ khid_bpf_ops->flags = uhid_bpf_ops->flags;
return 1;
}
return 0;
---
>
> Acked-by: Alexei Starovoitov <ast@xxxxxxxxxx>
And I'm going to apply the series now
Cheers,
Benjamin
