[PATCH 1/4] genl: Update genl refcount when creating/freeing a family

[PATCH 1/4] genl: Update genl refcount when creating/freeing a family

[Andrew Zaborowski]

[-- Attachment #1: Type: text/plain, Size: 748 bytes --]

Since struct l_genl is already refcounted we can hold references to the
l_genl object in an l_genl_family to allow the user to free them in
arbitrary order.
---
 ell/genl.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/ell/genl.c b/ell/genl.c
index 431ed05..ca4fa62 100644
--- a/ell/genl.c
+++ b/ell/genl.c
@@ -1800,7 +1800,7 @@ LIB_EXPORT struct l_genl_family *l_genl_family_new(struct l_genl *genl,
 	if (!entry)
 		return NULL;
 
-	family = family_alloc(genl, info->id);
+	family = family_alloc(l_genl_ref(genl), info->id);
 
 	return family;
 }
@@ -2059,4 +2059,5 @@ LIB_EXPORT void l_genl_family_free(struct l_genl_family *family)
 	}
 
 	l_free(family);
+	l_genl_unref(genl);
 }
-- 
2.20.1

[PATCH 2/4] genl: Export genl-private.h functions as public

[Andrew Zaborowski]

[-- Attachment #1: Type: text/plain, Size: 6767 bytes --]

Rename and upgrade _genl_msg_create and _genl_msg_as_bytes from
-private.h functions (for unit test use only) to public library
functions, specifically to be usable in IWD for encoding and decoding
raw genl messages.
---
 Makefile.am        |  1 -
 ell/ell.sym        |  2 ++
 ell/genl-private.h | 27 ----------------
 ell/genl.c         | 80 +++++++++++++++++++++++++++++-----------------
 ell/genl.h         |  5 +++
 5 files changed, 57 insertions(+), 58 deletions(-)
 delete mode 100644 ell/genl-private.h

diff --git a/Makefile.am b/Makefile.am
index 3ead678..8e025cb 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -83,7 +83,6 @@ ell_libell_la_SOURCES = $(linux_headers) \
 			ell/checksum.c \
 			ell/netlink-private.h \
 			ell/netlink.c \
-			ell/genl-private.h \
 			ell/genl.c \
 			ell/dbus-private.h \
 			ell/dbus.c \
diff --git a/ell/ell.sym b/ell/ell.sym
index 46df1bd..0c83b87 100644
--- a/ell/ell.sym
+++ b/ell/ell.sym
@@ -258,6 +258,8 @@ global:
 	l_genl_request_family;
 	l_genl_msg_new;
 	l_genl_msg_new_sized;
+	l_genl_msg_new_from_data;
+	l_genl_msg_to_data;
 	l_genl_msg_ref;
 	l_genl_msg_unref;
 	l_genl_msg_get_command;
diff --git a/ell/genl-private.h b/ell/genl-private.h
deleted file mode 100644
index 3c3e525..0000000
--- a/ell/genl-private.h
+++ /dev/null
@@ -1,27 +0,0 @@
-/*
- *
- *  Embedded Linux library
- *
- *  Copyright (C) 2011-2015  Intel Corporation. All rights reserved.
- *
- *  This library is free software; you can redistribute it and/or
- *  modify it under the terms of the GNU Lesser General Public
- *  License as published by the Free Software Foundation; either
- *  version 2.1 of the License, or (at your option) any later version.
- *
- *  This library is distributed in the hope that it will be useful,
- *  but WITHOUT ANY WARRANTY; without even the implied warranty of
- *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
- *  Lesser General Public License for more details.
- *
- *  You should have received a copy of the GNU Lesser General Public
- *  License along with this library; if not, write to the Free Software
- *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
- *
- */
-
-struct l_genl_msg *_genl_msg_create(const struct nlmsghdr *nlmsg);
-const void *_genl_msg_as_bytes(struct l_genl_msg *msg, uint16_t type,
-					uint16_t flags, uint32_t seq,
-					uint32_t pid,
-					size_t *out_size);
diff --git a/ell/genl.c b/ell/genl.c
index ca4fa62..815af90 100644
--- a/ell/genl.c
+++ b/ell/genl.c
@@ -34,7 +34,6 @@
 #include "io.h"
 #include "netlink-private.h"
 #include "genl.h"
-#include "genl-private.h"
 #include "private.h"
 
 #define MAX_NESTING_LEVEL 4
@@ -767,7 +766,7 @@ static bool msg_grow(struct l_genl_msg *msg, uint32_t needed)
 	return true;
 }
 
-struct l_genl_msg *_genl_msg_create(const struct nlmsghdr *nlmsg)
+static struct l_genl_msg *msg_create(const struct nlmsghdr *nlmsg)
 {
 	struct l_genl_msg *msg;
 
@@ -796,6 +795,32 @@ done:
 	return l_genl_msg_ref(msg);
 }
 
+static const void *msg_as_bytes(struct l_genl_msg *msg, uint16_t type,
+				uint16_t flags, uint32_t seq, uint32_t pid,
+				size_t *out_size)
+{
+	struct nlmsghdr *nlmsg;
+	struct genlmsghdr *genlmsg;
+
+	nlmsg = msg->data;
+
+	nlmsg->nlmsg_len = msg->len;
+	nlmsg->nlmsg_type = type;
+	nlmsg->nlmsg_flags = flags;
+	nlmsg->nlmsg_seq = seq;
+	nlmsg->nlmsg_pid = pid;
+
+	genlmsg = msg->data + NLMSG_HDRLEN;
+
+	genlmsg->cmd = msg->cmd;
+	genlmsg->version = msg->version;
+
+	if (out_size)
+		*out_size = msg->len;
+
+	return msg->data;
+}
+
 static void write_watch_destroy(void *user_data)
 {
 	struct l_genl *genl = user_data;
@@ -912,7 +937,7 @@ static void process_unicast(struct l_genl *genl, const struct nlmsghdr *nlmsg)
 					nlmsg->nlmsg_type == NLMSG_OVERRUN)
 		return;
 
-	msg = _genl_msg_create(nlmsg);
+	msg = msg_create(nlmsg);
 	if (!nlmsg->nlmsg_seq) {
 		if (msg)
 			dispatch_unicast_watches(genl, nlmsg->nlmsg_type, msg);
@@ -947,7 +972,7 @@ static void process_multicast(struct l_genl *genl, uint32_t group,
 						const struct nlmsghdr *nlmsg)
 {
 	const struct l_queue_entry *entry;
-	struct l_genl_msg *msg = _genl_msg_create(nlmsg);
+	struct l_genl_msg *msg = msg_create(nlmsg);
 
 	if (!msg)
 		return;
@@ -1492,41 +1517,36 @@ LIB_EXPORT bool l_genl_request_family(struct l_genl *genl, const char *name,
 	return false;
 }
 
-const void *_genl_msg_as_bytes(struct l_genl_msg *msg, uint16_t type,
-					uint16_t flags, uint32_t seq,
-					uint32_t pid,
-					size_t *out_size)
+LIB_EXPORT struct l_genl_msg *l_genl_msg_new(uint8_t cmd)
 {
-	struct nlmsghdr *nlmsg;
-	struct genlmsghdr *genlmsg;
-
-	nlmsg = msg->data;
-
-	nlmsg->nlmsg_len = msg->len;
-	nlmsg->nlmsg_type = type;
-	nlmsg->nlmsg_flags = flags;
-	nlmsg->nlmsg_seq = seq;
-	nlmsg->nlmsg_pid = pid;
+	return l_genl_msg_new_sized(cmd, 0);
+}
 
-	genlmsg = msg->data + NLMSG_HDRLEN;
+LIB_EXPORT struct l_genl_msg *l_genl_msg_new_sized(uint8_t cmd, uint32_t size)
+{
+	return msg_alloc(cmd, 0x00, size);
+}
 
-	genlmsg->cmd = msg->cmd;
-	genlmsg->version = msg->version;
+LIB_EXPORT struct l_genl_msg *l_genl_msg_new_from_data(const void *data,
+							size_t size)
+{
+	const struct nlmsghdr *nlmsg = (const void *) data;
 
-	if (out_size)
-		*out_size = msg->len;
+	if (size < sizeof(struct nlmsghdr))
+		return NULL;
 
-	return msg->data;
-}
+	if (size < nlmsg->nlmsg_len)
+		return NULL;
 
-LIB_EXPORT struct l_genl_msg *l_genl_msg_new(uint8_t cmd)
-{
-	return l_genl_msg_new_sized(cmd, 0);
+	return msg_create(nlmsg);
 }
 
-LIB_EXPORT struct l_genl_msg *l_genl_msg_new_sized(uint8_t cmd, uint32_t size)
+LIB_EXPORT const void *l_genl_msg_to_data(struct l_genl_msg *msg, uint16_t type,
+						uint16_t flags, uint32_t seq,
+						uint32_t pid,
+						size_t *out_size)
 {
-	return msg_alloc(cmd, 0x00, size);
+	return msg_as_bytes(msg, type, flags, seq, pid, out_size);
 }
 
 LIB_EXPORT struct l_genl_msg *l_genl_msg_ref(struct l_genl_msg *msg)
diff --git a/ell/genl.h b/ell/genl.h
index c3f641f..312a6b2 100644
--- a/ell/genl.h
+++ b/ell/genl.h
@@ -85,6 +85,11 @@ struct l_genl_attr {
 
 struct l_genl_msg* l_genl_msg_new(uint8_t cmd);
 struct l_genl_msg *l_genl_msg_new_sized(uint8_t cmd, uint32_t size);
+struct l_genl_msg *l_genl_msg_new_from_data(const void *data, size_t size);
+
+const void *l_genl_msg_to_data(struct l_genl_msg *msg, uint16_t type,
+				uint16_t flags, uint32_t seq, uint32_t pid,
+				size_t *out_size);
 
 struct l_genl_msg *l_genl_msg_ref(struct l_genl_msg *msg);
 void l_genl_msg_unref(struct l_genl_msg *msg);
-- 
2.20.1

[PATCH 3/4] genl: Take advantage of msg_as_bytes in can_write_data

[Andrew Zaborowski]

[-- Attachment #1: Type: text/plain, Size: 1406 bytes --]

---
 ell/genl.c | 22 +++++-----------------
 1 file changed, 5 insertions(+), 17 deletions(-)

diff --git a/ell/genl.c b/ell/genl.c
index 815af90..f65263d 100644
--- a/ell/genl.c
+++ b/ell/genl.c
@@ -832,8 +832,8 @@ static bool can_write_data(struct l_io *io, void *user_data)
 {
 	struct l_genl *genl = user_data;
 	struct genl_request *request;
-	struct nlmsghdr *nlmsg;
-	struct genlmsghdr *genlmsg;
+	const void *data;
+	size_t size;
 	ssize_t bytes_written;
 
 	request = l_queue_pop_head(genl->request_queue);
@@ -841,22 +841,10 @@ static bool can_write_data(struct l_io *io, void *user_data)
 		return false;
 
 	request->seq = get_next_id(&genl->next_seq);
+	data = msg_as_bytes(request->msg, request->type, request->flags,
+				request->seq, genl->pid, &size);
 
-	nlmsg = request->msg->data;
-
-	nlmsg->nlmsg_len = request->msg->len;
-	nlmsg->nlmsg_type = request->type;
-	nlmsg->nlmsg_flags = request->flags;
-	nlmsg->nlmsg_seq = request->seq;
-	nlmsg->nlmsg_pid = genl->pid;
-
-	genlmsg = request->msg->data + NLMSG_HDRLEN;
-
-	genlmsg->cmd = request->msg->cmd;
-	genlmsg->version = request->msg->version;
-
-	bytes_written = send(genl->fd, request->msg->data,
-						request->msg->len, 0);
+	bytes_written = send(genl->fd, data, size, 0);
 	if (bytes_written < 0) {
 		l_queue_push_head(genl->request_queue, request);
 		return false;
-- 
2.20.1

[PATCH 4/4] unit: Update calls to functions from genl-private.h

[Andrew Zaborowski]

[-- Attachment #1: Type: text/plain, Size: 2620 bytes --]

---
 unit/test-genl-msg.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/unit/test-genl-msg.c b/unit/test-genl-msg.c
index b1c5275..119d01e 100644
--- a/unit/test-genl-msg.c
+++ b/unit/test-genl-msg.c
@@ -29,8 +29,6 @@
 #include <linux/genetlink.h>
 #include <ell/ell.h>
 
-#include "ell/genl-private.h"
-
 static bool do_print = false;
 
 static void do_debug(const char *str, void *user_data)
@@ -61,7 +59,7 @@ static void parse_set_station(const void *data)
 	const void *payload;
 
 	nlmsg = (struct nlmsghdr *) set_station_request;
-	msg = _genl_msg_create(nlmsg);
+	msg = l_genl_msg_new_from_data(nlmsg, sizeof(set_station_request));
 	assert(msg);
 
 	assert(l_genl_msg_get_command(msg) == 18);
@@ -113,7 +111,7 @@ static void build_set_station(const void *data)
 	assert(l_genl_msg_append_attr(msg, 6, 6, mac));
 	assert(l_genl_msg_append_attr(msg, 67, 8, flags));
 
-	raw = _genl_msg_as_bytes(msg, 0x17, 0x05, 0x550d538b, 3604, &size);
+	raw = l_genl_msg_to_data(msg, 0x17, 0x05, 0x550d538b, 3604, &size);
 
 	if (do_print) {
 		l_util_hexdump(false, raw, size, do_debug, "[MSG] ");
@@ -157,7 +155,8 @@ static void parse_set_rekey_offload(const void *data)
 	const void *payload;
 
 	nlmsg = (struct nlmsghdr *) set_rekey_offload_request;
-	msg = _genl_msg_create(nlmsg);
+	msg = l_genl_msg_new_from_data(nlmsg,
+					sizeof(set_rekey_offload_request));
 	assert(msg);
 
 	assert(l_genl_msg_get_command(msg) == 79);
@@ -230,7 +229,7 @@ static void build_set_rekey_offload(const void *data)
 	assert(l_genl_msg_append_attr(msg, 3, 8, replay_counter));
 	assert(l_genl_msg_leave_nested(msg));
 
-	raw = _genl_msg_as_bytes(msg, 0x1b, 0x05, 0x53e1a359, 0xe74002ba,
+	raw = l_genl_msg_to_data(msg, 0x1b, 0x05, 0x53e1a359, 0xe74002ba,
 					&size);
 	if (do_print) {
 		l_util_hexdump(false, raw, size, do_debug, "[MSG] ");
@@ -285,7 +284,7 @@ static void parse_libnl_nested(const void *data)
 	const void *payload;
 
 	nlmsg = (struct nlmsghdr *) libnl_nested;
-	msg = _genl_msg_create(nlmsg);
+	msg = l_genl_msg_new_from_data(nlmsg, sizeof(libnl_nested));
 	assert(msg);
 
 	assert(l_genl_msg_get_command(msg) == 1);
@@ -353,7 +352,7 @@ static void build_libnl_nested(const void *data)
 	assert(l_genl_msg_leave_nested(msg));
 	assert(l_genl_msg_leave_nested(msg));
 
-	raw = _genl_msg_as_bytes(msg, 0x15, 0x05, 0x55130572, 0x0c406877,
+	raw = l_genl_msg_to_data(msg, 0x15, 0x05, 0x55130572, 0x0c406877,
 					&size);
 	if (do_print) {
 		l_util_hexdump(false, raw, size, do_debug, "[MSG] ");
-- 
2.20.1

Re: [PATCH 1/4] genl: Update genl refcount when creating/freeing a family

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 357 bytes --]

Hi Andrew,

On 2/14/20 5:37 PM, Andrew Zaborowski wrote:
> Since struct l_genl is already refcounted we can hold references to the
> l_genl object in an l_genl_family to allow the user to free them in
> arbitrary order.
> ---
>   ell/genl.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 

All applied, thanks.

Regards,
-Denis