Re: [PATCH net-next 07/10] genetlink: add genlmsg_iput() API

[Jiri Pirko <jiri@resnulli.us>]

Wed, Aug 09, 2023 at 08:26:45PM CEST, kuba@kernel.org wrote:

[...]

>@@ -270,6 +270,31 @@ genl_info_dump(struct netlink_callback *cb)
> 	return &genl_dumpit_info(cb)->info;
> }
> 
>+#ifdef __LITTLE_ENDIAN
>+#define __GENL_PTR_LOW(byte)	((void *)(unsigned long)(byte))
>+#else
>+#define __GENL_PTR_LOW(byte)	\
>+	((void *)((unsigned long)(byte) << (BITS_PER_LONG - 8)))
>+#endif
>+
>+/**
>+ * GENL_INFO_NTF() - define genl_info for notifications
>+ * @__name: name of declared variable
>+ * @__family: pointer to the genetlink family
>+ * @__cmd: command to be used in the notification
>+ */
>+#define GENL_INFO_NTF(__name, __family, __cmd)			\
>+	struct genl_info __name = {				\
>+		.family = (__family),				\
>+		.genlhdr = (void *)&(__name.user_ptr[0]),	\
>+		.user_ptr[0] = __GENL_PTR_LOW(__cmd),		\

Ugh. Took me some time to decypher what you do here. Having endian
specific code here seems quite odd to me. Why don't you have this as
static inline initializer function instead and use struct genlmsghdr
pointer to store cmd where it belong?

static inline void genl_info_ntf(struct genl_info *info,
				 const struct genl_family *family, u8 cmd)
}
	struct genlmsghdr *hdr = (void *) &info->user_ptr[0];

	info->family = family;
	info->genlhdr = hdr;
	hdr->cmd = cmd;
}

[...]