[PATCH libmnl] libmnl: add support for signed types

[PATCH libmnl] libmnl: add support for signed types

[Jacob Keller]

libmnl has get and put functions for unsigned integer types. It lacks
support for the signed variations. On some level this is technically
sufficient. A user could use the unsigned variations and then cast to a
signed value at use. However, this makes resulting code in the application
more difficult to follow. Introduce signed variations of the integer get
and put functions.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
 include/libmnl/libmnl.h |  16 ++++
 src/attr.c              | 194 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 210 insertions(+)

diff --git a/include/libmnl/libmnl.h b/include/libmnl/libmnl.h
index 4bd0b92e8742..6c37cd532a3d 100644
--- a/include/libmnl/libmnl.h
+++ b/include/libmnl/libmnl.h
@@ -92,6 +92,10 @@ extern uint8_t mnl_attr_get_u8(const struct nlattr *attr);
 extern uint16_t mnl_attr_get_u16(const struct nlattr *attr);
 extern uint32_t mnl_attr_get_u32(const struct nlattr *attr);
 extern uint64_t mnl_attr_get_u64(const struct nlattr *attr);
+extern int8_t mnl_attr_get_s8(const struct nlattr *attr);
+extern int16_t mnl_attr_get_s16(const struct nlattr *attr);
+extern int32_t mnl_attr_get_s32(const struct nlattr *attr);
+extern int64_t mnl_attr_get_s64(const struct nlattr *attr);
 extern const char *mnl_attr_get_str(const struct nlattr *attr);
 
 /* TLV attribute putters */
@@ -100,6 +104,10 @@ extern void mnl_attr_put_u8(struct nlmsghdr *nlh, uint16_t type, uint8_t data);
 extern void mnl_attr_put_u16(struct nlmsghdr *nlh, uint16_t type, uint16_t data);
 extern void mnl_attr_put_u32(struct nlmsghdr *nlh, uint16_t type, uint32_t data);
 extern void mnl_attr_put_u64(struct nlmsghdr *nlh, uint16_t type, uint64_t data);
+extern void mnl_attr_put_s8(struct nlmsghdr *nlh, uint16_t type, int8_t data);
+extern void mnl_attr_put_s16(struct nlmsghdr *nlh, uint16_t type, int16_t data);
+extern void mnl_attr_put_s32(struct nlmsghdr *nlh, uint16_t type, int32_t data);
+extern void mnl_attr_put_s64(struct nlmsghdr *nlh, uint16_t type, int64_t data);
 extern void mnl_attr_put_str(struct nlmsghdr *nlh, uint16_t type, const char *data);
 extern void mnl_attr_put_strz(struct nlmsghdr *nlh, uint16_t type, const char *data);
 
@@ -109,6 +117,10 @@ extern bool mnl_attr_put_u8_check(struct nlmsghdr *nlh, size_t buflen, uint16_t
 extern bool mnl_attr_put_u16_check(struct nlmsghdr *nlh, size_t buflen, uint16_t type, uint16_t data);
 extern bool mnl_attr_put_u32_check(struct nlmsghdr *nlh, size_t buflen, uint16_t type, uint32_t data);
 extern bool mnl_attr_put_u64_check(struct nlmsghdr *nlh, size_t buflen, uint16_t type, uint64_t data);
+extern bool mnl_attr_put_s8_check(struct nlmsghdr *nlh, size_t buflen, uint16_t type, int8_t data);
+extern bool mnl_attr_put_s16_check(struct nlmsghdr *nlh, size_t buflen, uint16_t type, int16_t data);
+extern bool mnl_attr_put_s32_check(struct nlmsghdr *nlh, size_t buflen, uint16_t type, int32_t data);
+extern bool mnl_attr_put_s64_check(struct nlmsghdr *nlh, size_t buflen, uint16_t type, int64_t data);
 extern bool mnl_attr_put_str_check(struct nlmsghdr *nlh, size_t buflen, uint16_t type, const char *data);
 extern bool mnl_attr_put_strz_check(struct nlmsghdr *nlh, size_t buflen, uint16_t type, const char *data);
 
@@ -127,6 +139,10 @@ enum mnl_attr_data_type {
 	MNL_TYPE_U16,
 	MNL_TYPE_U32,
 	MNL_TYPE_U64,
+	MNL_TYPE_S8,
+	MNL_TYPE_S16,
+	MNL_TYPE_S32,
+	MNL_TYPE_S64,
 	MNL_TYPE_STRING,
 	MNL_TYPE_FLAG,
 	MNL_TYPE_MSECS,
diff --git a/src/attr.c b/src/attr.c
index 838eab063981..cf971d6ed3c1 100644
--- a/src/attr.c
+++ b/src/attr.c
@@ -192,6 +192,10 @@ static const size_t mnl_attr_data_type_len[MNL_TYPE_MAX] = {
 	[MNL_TYPE_U16]		= sizeof(uint16_t),
 	[MNL_TYPE_U32]		= sizeof(uint32_t),
 	[MNL_TYPE_U64]		= sizeof(uint64_t),
+	[MNL_TYPE_S8]		= sizeof(int8_t),
+	[MNL_TYPE_S16]		= sizeof(int16_t),
+	[MNL_TYPE_S32]		= sizeof(int32_t),
+	[MNL_TYPE_S64]		= sizeof(int64_t),
 	[MNL_TYPE_MSECS]	= sizeof(uint64_t),
 };
 
@@ -371,6 +375,54 @@ EXPORT_SYMBOL uint64_t mnl_attr_get_u64(const struct nlattr *attr)
 	return tmp;
 }
 
+/**
+ * mnl_attr_get_s8 - returns 8-bit signed integer attribute payload
+ * \param attr pointer to netlink attribute
+ *
+ * This function returns the 8-bit value of the attribute payload.
+ */
+EXPORT_SYMBOL int8_t mnl_attr_get_s8(const struct nlattr *attr)
+{
+	return *((int8_t *)mnl_attr_get_payload(attr));
+}
+
+/**
+ * mnl_attr_get_s16 - returns 16-bit signed integer attribute payload
+ * \param attr pointer to netlink attribute
+ *
+ * This function returns the 16-bit value of the attribute payload.
+ */
+EXPORT_SYMBOL int16_t mnl_attr_get_s16(const struct nlattr *attr)
+{
+	return *((int16_t *)mnl_attr_get_payload(attr));
+}
+
+/**
+ * mnl_attr_get_s32 - returns 32-bit signed integer attribute payload
+ * \param attr pointer to netlink attribute
+ *
+ * This function returns the 32-bit value of the attribute payload.
+ */
+EXPORT_SYMBOL int32_t mnl_attr_get_s32(const struct nlattr *attr)
+{
+	return *((int32_t *)mnl_attr_get_payload(attr));
+}
+
+/**
+ * mnl_attr_get_s64 - returns 64-bit signed integer attribute.
+ * \param attr pointer to netlink attribute
+ *
+ * This function returns the 64-bit value of the attribute payload. This
+ * function is align-safe, since accessing 64-bit Netlink attributes is a
+ * common source of alignment issues.
+ */
+EXPORT_SYMBOL int64_t mnl_attr_get_s64(const struct nlattr *attr)
+{
+	int64_t tmp;
+	memcpy(&tmp, mnl_attr_get_payload(attr), sizeof(tmp));
+	return tmp;
+}
+
 /**
  * mnl_attr_get_str - returns pointer to string attribute.
  * \param attr pointer to netlink attribute
@@ -469,6 +521,66 @@ EXPORT_SYMBOL void mnl_attr_put_u64(struct nlmsghdr *nlh, uint16_t type,
 	mnl_attr_put(nlh, type, sizeof(uint64_t), &data);
 }
 
+/**
+ * mnl_attr_put_s8 - add 8-bit signed integer attribute to netlink message
+ * \param nlh pointer to the netlink message
+ * \param type netlink attribute type
+ * \param data 8-bit signed integer data that is stored by the new attribute
+ *
+ * This function updates the length field of the Netlink message (nlmsg_len)
+ * by adding the size (header + payload) of the new attribute.
+ */
+EXPORT_SYMBOL void mnl_attr_put_s8(struct nlmsghdr *nlh, uint16_t type,
+				   int8_t data)
+{
+	mnl_attr_put(nlh, type, sizeof(int8_t), &data);
+}
+
+/**
+ * mnl_attr_put_s16 - add 16-bit signed integer attribute to netlink message
+ * \param nlh pointer to the netlink message
+ * \param type netlink attribute type
+ * \param data 16-bit signed integer data that is stored by the new attribute
+ *
+ * This function updates the length field of the Netlink message (nlmsg_len)
+ * by adding the size (header + payload) of the new attribute.
+ */
+EXPORT_SYMBOL void mnl_attr_put_s16(struct nlmsghdr *nlh, uint16_t type,
+				    int16_t data)
+{
+	mnl_attr_put(nlh, type, sizeof(int16_t), &data);
+}
+
+/**
+ * mnl_attr_put_s32 - add 32-bit signed integer attribute to netlink message
+ * \param nlh pointer to the netlink message
+ * \param type netlink attribute type
+ * \param data 32-bit signed integer data that is stored by the new attribute
+ *
+ * This function updates the length field of the Netlink message (nlmsg_len)
+ * by adding the size (header + payload) of the new attribute.
+ */
+EXPORT_SYMBOL void mnl_attr_put_s32(struct nlmsghdr *nlh, uint16_t type,
+				    int32_t data)
+{
+	mnl_attr_put(nlh, type, sizeof(int32_t), &data);
+}
+
+/**
+ * mnl_attr_put_s64 - add 64-bit signed integer attribute to netlink message
+ * \param nlh pointer to the netlink message
+ * \param type netlink attribute type
+ * \param data 64-bit signed integer data that is stored by the new attribute
+ *
+ * This function updates the length field of the Netlink message (nlmsg_len)
+ * by adding the size (header + payload) of the new attribute.
+ */
+EXPORT_SYMBOL void mnl_attr_put_s64(struct nlmsghdr *nlh, uint16_t type,
+				    int64_t data)
+{
+	mnl_attr_put(nlh, type, sizeof(int64_t), &data);
+}
+
 /**
  * mnl_attr_put_str - add string attribute to netlink message
  * \param nlh  pointer to the netlink message
@@ -629,6 +741,88 @@ EXPORT_SYMBOL bool mnl_attr_put_u64_check(struct nlmsghdr *nlh, size_t buflen,
 	return mnl_attr_put_check(nlh, buflen, type, sizeof(uint64_t), &data);
 }
 
+/**
+ * mnl_attr_put_s8_check - add 8-bit signed int attribute to netlink message
+ * \param nlh pointer to the netlink message
+ * \param buflen size of buffer which stores the message
+ * \param type netlink attribute type
+ * \param data 8-bit signed integer data that is stored by the new attribute
+ *
+ * This function first checks that the data can be added to the message
+ * (fits into the buffer) and then updates the length field of the Netlink
+ * message (nlmsg_len) by adding the size (header + payload) of the new
+ * attribute. The function returns true if the attribute could be added
+ * to the message, otherwise false is returned.
+ */
+EXPORT_SYMBOL bool mnl_attr_put_s8_check(struct nlmsghdr *nlh, size_t buflen,
+					 uint16_t type, int8_t data)
+{
+	return mnl_attr_put_check(nlh, buflen, type, sizeof(data), &data);
+}
+
+/**
+ * mnl_attr_put_s16_check - add 16-bit signed int attribute to netlink message
+ * \param nlh pointer to the netlink message
+ * \param buflen size of buffer which stores the message
+ * \param type netlink attribute type
+ * \param data 16-bit signed integer data that is stored by the new attribute
+ *
+ * This function first checks that the data can be added to the message
+ * (fits into the buffer) and then updates the length field of the Netlink
+ * message (nlmsg_len) by adding the size (header + payload) of the new
+ * attribute. The function returns true if the attribute could be added
+ * to the message, otherwise false is returned.
+ * This function updates the length field of the Netlink message (nlmsg_len)
+ * by adding the size (header + payload) of the new attribute.
+ */
+EXPORT_SYMBOL bool mnl_attr_put_s16_check(struct nlmsghdr *nlh, size_t buflen,
+					  uint16_t type, int16_t data)
+{
+	return mnl_attr_put_check(nlh, buflen, type, sizeof(data), &data);
+}
+
+/**
+ * mnl_attr_put_s32_check - add 32-bit signed int attribute to netlink message
+ * \param nlh pointer to the netlink message
+ * \param buflen size of buffer which stores the message
+ * \param type netlink attribute type
+ * \param data 32-bit signed integer data that is stored by the new attribute
+ *
+ * This function first checks that the data can be added to the message
+ * (fits into the buffer) and then updates the length field of the Netlink
+ * message (nlmsg_len) by adding the size (header + payload) of the new
+ * attribute. The function returns true if the attribute could be added
+ * to the message, otherwise false is returned.
+ * This function updates the length field of the Netlink message (nlmsg_len)
+ * by adding the size (header + payload) of the new attribute.
+ */
+EXPORT_SYMBOL bool mnl_attr_put_s32_check(struct nlmsghdr *nlh, size_t buflen,
+					  uint16_t type, int32_t data)
+{
+	return mnl_attr_put_check(nlh, buflen, type, sizeof(data), &data);
+}
+
+/**
+ * mnl_attr_put_s64_check - add 64-bit signed int attribute to netlink message
+ * \param nlh pointer to the netlink message
+ * \param buflen size of buffer which stores the message
+ * \param type netlink attribute type
+ * \param data 64-bit signed integer data that is stored by the new attribute
+ *
+ * This function first checks that the data can be added to the message
+ * (fits into the buffer) and then updates the length field of the Netlink
+ * message (nlmsg_len) by adding the size (header + payload) of the new
+ * attribute. The function returns true if the attribute could be added
+ * to the message, otherwise false is returned.
+ * This function updates the length field of the Netlink message (nlmsg_len)
+ * by adding the size (header + payload) of the new attribute.
+ */
+EXPORT_SYMBOL bool mnl_attr_put_s64_check(struct nlmsghdr *nlh, size_t buflen,
+					  uint16_t type, int64_t data)
+{
+	return mnl_attr_put_check(nlh, buflen, type, sizeof(data), &data);
+}
+
 /**
  * mnl_attr_put_str_check - add string attribute to netlink message
  * \param nlh  pointer to the netlink message
-- 
2.37.1.208.ge72d93e88cb2

Re: [PATCH libmnl] libmnl: add support for signed types

[Jan Engelhardt]

>+ * mnl_attr_get_s8 - returns 8-bit signed integer attribute payload
>+ *
>+ * This function returns the 8-bit value of the attribute payload.
>+ */

That's kinda redundant - it's logically the same thing, just written 
ever-so-slightly differently.


>+/**
>+ * mnl_attr_get_s64 - returns 64-bit signed integer attribute.
>    This
>+ * function is align-safe, since accessing 64-bit Netlink attributes is a
>+ * common source of alignment issues.

That sentence is self-defeating. If NLA access is a commn source of 
alignment issues, then, by transitivity, this function too would
be potentially affected. Just

  "This function reads the 64-bit nlattr in an alignment-safe manner."

RE: [PATCH libmnl] libmnl: add support for signed types

[Keller, Jacob E]

> -----Original Message-----
> From: Jan Engelhardt <jengelh@inai.de>
> Sent: Thursday, August 04, 2022 4:36 PM
> To: Keller, Jacob E <jacob.e.keller@intel.com>
> Cc: Netfilter Devel <netfilter-devel@vger.kernel.org>; Duncan Roe
> <duncan_roe@optusnet.com.au>; Pablo Neira Ayuso <pablo@netfilter.org>
> Subject: Re: [PATCH libmnl] libmnl: add support for signed types
> 
> 
> >+ * mnl_attr_get_s8 - returns 8-bit signed integer attribute payload
> >+ *
> >+ * This function returns the 8-bit value of the attribute payload.
> >+ */
> 
> That's kinda redundant - it's logically the same thing, just written
> ever-so-slightly differently.
> 

I guess I can fix the unsigned wording as well then. This is just a copy of that but converted to signed.

> 
> >+/**
> >+ * mnl_attr_get_s64 - returns 64-bit signed integer attribute.
> >    This
> >+ * function is align-safe, since accessing 64-bit Netlink attributes is a
> >+ * common source of alignment issues.
> 
> That sentence is self-defeating. If NLA access is a commn source of
> alignment issues, then, by transitivity, this function too would
> be potentially affected. Just
> 

This is specifically calling out the use of memcpy rather than just dereferencing the pointer address. The default NLA alignment is only 4bytes, not 8 bytes. This is exactly the same as the unsigned implementation, just copied to implement support for signed types.

Thanks,
Jake

>   "This function reads the 64-bit nlattr in an alignment-safe manner."
> 

Re: [PATCH libmnl] libmnl: add support for signed types

[Duncan Roe]

Hi Jacob,

On Fri, Aug 05, 2022 at 01:35:45AM +0200, Jan Engelhardt wrote:
>
> >+ * mnl_attr_get_s8 - returns 8-bit signed integer attribute payload
> >+ *
> >+ * This function returns the 8-bit value of the attribute payload.
> >+ */
>
> That's kinda redundant - it's logically the same thing, just written
> ever-so-slightly differently.
>
>
> >+/**
> >+ * mnl_attr_get_s64 - returns 64-bit signed integer attribute.
> >    This
> >+ * function is align-safe, since accessing 64-bit Netlink attributes is a
> >+ * common source of alignment issues.
>
> That sentence is self-defeating. If NLA access is a commn source of
> alignment issues, then, by transitivity, this function too would
> be potentially affected. Just
>
>   "This function reads the 64-bit nlattr in an alignment-safe manner."
>
>
Please use the following documentation style for *all* new functions. The man
pages look better that way:

>  * mnl_attr_get_s8 - get 8-bit signed integer attribute payload
>  * \param attr pointer to netlink attribute
>  *
>  * \return 8-bit value of the attribute payload

I.e. "get" instead of "returns" in 1-line description; Use \return instead of
"This function returns the"; No full stop at end of \return description.

Cheers ... Duncan.