[PATCH v2 0/5] selftests/kdbus: small fixes

[PATCH v2 0/5] selftests/kdbus: small fixes

[Sergei Zviagintsev]

Set of different fixes over selftests/kdbus.

These were first sent as independent patches, and in v2 they just
properly threaded.

Links to previous patches:

http://lkml.kernel.org/g/1434550697-24785-1-git-send-email-sergei@s15v.net
http://lkml.kernel.org/g/1434550697-24785-2-git-send-email-sergei@s15v.net
http://lkml.kernel.org/g/1434550697-24785-3-git-send-email-sergei@s15v.net
http://lkml.kernel.org/g/1434550697-24785-4-git-send-email-sergei@s15v.net
http://lkml.kernel.org/g/1434550697-24785-5-git-send-email-sergei@s15v.net

Sergei Zviagintsev (5):
  selftests/kdbus: handle cap_get_proc() error properly
  selftests/kdbus: drop useless assignment
  selftests/kdbus: remove useless initializations from
    kdbus_clone_userns_test()
  selftests/kdbus: drop duplicated code from __kdbus_msg_send()
  selftests/kdbus: fix error paths in __kdbus_msg_send()

 tools/testing/selftests/kdbus/kdbus-util.c       | 46 ++++++++++++------------
 tools/testing/selftests/kdbus/test-metadata-ns.c |  9 ++---
 2 files changed, 27 insertions(+), 28 deletions(-)

-- 
1.8.3.1

[PATCH v2 1/5] selftests/kdbus: handle cap_get_proc() error properly

[Sergei Zviagintsev]

Fix typo in checking error value of cap_get_proc(): cap -> caps

Signed-off-by: Sergei Zviagintsev <sergei@s15v.net>
---
 tools/testing/selftests/kdbus/kdbus-util.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/kdbus/kdbus-util.c b/tools/testing/selftests/kdbus/kdbus-util.c
index 1e267d585a14..a3e49b04f3a7 100644
--- a/tools/testing/selftests/kdbus/kdbus-util.c
+++ b/tools/testing/selftests/kdbus/kdbus-util.c
@@ -1547,7 +1547,7 @@ int test_is_capable(int cap, ...)
 	cap_t caps;
 
 	caps = cap_get_proc();
-	if (!cap) {
+	if (!caps) {
 		ret = -errno;
 		kdbus_printf("error cap_get_proc(): %d (%m)\n", ret);
 		return ret;
-- 
1.8.3.1

[PATCH v2 2/5] selftests/kdbus: drop useless assignment

[Sergei Zviagintsev]

Assign returned file descriptor directly to `fd', without intermediate
`ret' variable.

Signed-off-by: Sergei Zviagintsev <sergei@s15v.net>
---
 tools/testing/selftests/kdbus/kdbus-util.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/tools/testing/selftests/kdbus/kdbus-util.c b/tools/testing/selftests/kdbus/kdbus-util.c
index a3e49b04f3a7..f9102264cfb9 100644
--- a/tools/testing/selftests/kdbus/kdbus-util.c
+++ b/tools/testing/selftests/kdbus/kdbus-util.c
@@ -408,11 +408,9 @@ int sys_memfd_create(const char *name, __u64 size)
 {
 	int ret, fd;
 
-	ret = syscall(__NR_memfd_create, name, MFD_ALLOW_SEALING);
-	if (ret < 0)
-		return ret;
-
-	fd = ret;
+	fd = syscall(__NR_memfd_create, name, MFD_ALLOW_SEALING);
+	if (fd < 0)
+		return fd;
 
 	ret = ftruncate(fd, size);
 	if (ret < 0) {
-- 
1.8.3.1

[PATCH v2 3/5] selftests/kdbus: remove useless initializations from kdbus_clone_userns_test()

[Sergei Zviagintsev]

Do not initialize efd, unpriv_conn_id, userns_conn_id and monitor. These
vars are assigned to values later in code while initial values are never
used.

Signed-off-by: Sergei Zviagintsev <sergei@s15v.net>
---
 tools/testing/selftests/kdbus/test-metadata-ns.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/kdbus/test-metadata-ns.c b/tools/testing/selftests/kdbus/test-metadata-ns.c
index 2cb1d4d2a5be..ccdfae06922b 100644
--- a/tools/testing/selftests/kdbus/test-metadata-ns.c
+++ b/tools/testing/selftests/kdbus/test-metadata-ns.c
@@ -281,16 +281,13 @@ out:
 static int kdbus_clone_userns_test(const char *bus,
 				   struct kdbus_conn *conn)
 {
-	int ret;
-	int status;
-	int efd = -1;
+	int ret, status, efd;
 	pid_t pid, ppid;
-	uint64_t unpriv_conn_id = 0;
-	uint64_t userns_conn_id = 0;
+	uint64_t unpriv_conn_id, userns_conn_id;
 	struct kdbus_msg *msg;
 	const struct kdbus_item *item;
 	struct kdbus_pids expected_pids;
-	struct kdbus_conn *monitor = NULL;
+	struct kdbus_conn *monitor;
 
 	kdbus_printf("STARTING TEST 'metadata-ns'.\n");
 
-- 
1.8.3.1

[PATCH v2 4/5] selftests/kdbus: drop duplicated code from __kdbus_msg_send()

[Sergei Zviagintsev]

Set value of `size' in one step instead of four.

Signed-off-by: Sergei Zviagintsev <sergei@s15v.net>
---
 tools/testing/selftests/kdbus/kdbus-util.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/tools/testing/selftests/kdbus/kdbus-util.c b/tools/testing/selftests/kdbus/kdbus-util.c
index f9102264cfb9..ae81da63d810 100644
--- a/tools/testing/selftests/kdbus/kdbus-util.c
+++ b/tools/testing/selftests/kdbus/kdbus-util.c
@@ -462,10 +462,7 @@ static int __kdbus_msg_send(const struct kdbus_conn *conn,
 	int memfd = -1;
 	int ret;
 
-	size = sizeof(*msg);
-	size += KDBUS_ITEM_SIZE(sizeof(struct kdbus_vec));
-	size += KDBUS_ITEM_SIZE(sizeof(struct kdbus_vec));
-	size += KDBUS_ITEM_SIZE(sizeof(struct kdbus_vec));
+	size = sizeof(*msg) + 3 * KDBUS_ITEM_SIZE(sizeof(struct kdbus_vec));
 
 	if (dst_id == KDBUS_DST_ID_BROADCAST)
 		size += KDBUS_ITEM_SIZE(sizeof(struct kdbus_bloom_filter)) + 64;
-- 
1.8.3.1

[PATCH v2 5/5] selftests/kdbus: fix error paths in __kdbus_msg_send()

[Sergei Zviagintsev]

Handle errors properly, free allocated resources.

Signed-off-by: Sergei Zviagintsev <sergei@s15v.net>
---
 tools/testing/selftests/kdbus/kdbus-util.c | 31 ++++++++++++++++++------------
 1 file changed, 19 insertions(+), 12 deletions(-)

diff --git a/tools/testing/selftests/kdbus/kdbus-util.c b/tools/testing/selftests/kdbus/kdbus-util.c
index ae81da63d810..29a0cb1aace2 100644
--- a/tools/testing/selftests/kdbus/kdbus-util.c
+++ b/tools/testing/selftests/kdbus/kdbus-util.c
@@ -452,8 +452,8 @@ static int __kdbus_msg_send(const struct kdbus_conn *conn,
 			    uint64_t cmd_flags,
 			    int cancel_fd)
 {
-	struct kdbus_cmd_send *cmd;
-	struct kdbus_msg *msg;
+	struct kdbus_cmd_send *cmd = NULL;
+	struct kdbus_msg *msg = NULL;
 	const char ref1[1024 * 128 + 3] = "0123456789_0";
 	const char ref2[] = "0123456789_1";
 	struct kdbus_item *item;
@@ -476,14 +476,14 @@ static int __kdbus_msg_send(const struct kdbus_conn *conn,
 		if (write(memfd, "kdbus memfd 1234567", 19) != 19) {
 			ret = -errno;
 			kdbus_printf("writing to memfd failed: %m\n");
-			return ret;
+			goto out;
 		}
 
 		ret = sys_memfd_seal_set(memfd);
 		if (ret < 0) {
 			ret = -errno;
 			kdbus_printf("memfd sealing failed: %m\n");
-			return ret;
+			goto out;
 		}
 
 		size += KDBUS_ITEM_SIZE(sizeof(struct kdbus_memfd));
@@ -496,7 +496,7 @@ static int __kdbus_msg_send(const struct kdbus_conn *conn,
 	if (!msg) {
 		ret = -errno;
 		kdbus_printf("unable to malloc()!?\n");
-		return ret;
+		goto out;
 	}
 
 	if (dst_id == KDBUS_DST_ID_BROADCAST)
@@ -514,7 +514,7 @@ static int __kdbus_msg_send(const struct kdbus_conn *conn,
 	if (timeout) {
 		ret = clock_gettime(CLOCK_MONOTONIC_COARSE, &now);
 		if (ret < 0)
-			return ret;
+			goto out;
 
 		msg->timeout_ns = now.tv_sec * 1000000000ULL +
 				  now.tv_nsec + timeout;
@@ -565,6 +565,12 @@ static int __kdbus_msg_send(const struct kdbus_conn *conn,
 		size += KDBUS_ITEM_SIZE(sizeof(cancel_fd));
 
 	cmd = malloc(size);
+	if (!cmd) {
+		ret = -errno;
+		kdbus_printf("unable to malloc()!?\n");
+		goto out;
+	}
+
 	cmd->size = size;
 	cmd->flags = cmd_flags;
 	cmd->msg_address = (uintptr_t)msg;
@@ -579,12 +585,9 @@ static int __kdbus_msg_send(const struct kdbus_conn *conn,
 	}
 
 	ret = kdbus_cmd_send(conn->fd, cmd);
-	if (memfd >= 0)
-		close(memfd);
-
 	if (ret < 0) {
 		kdbus_printf("error sending message: %d (%m)\n", ret);
-		return ret;
+		goto out;
 	}
 
 	if (cmd_flags & KDBUS_SEND_SYNC_REPLY) {
@@ -598,13 +601,17 @@ static int __kdbus_msg_send(const struct kdbus_conn *conn,
 
 		ret = kdbus_free(conn, cmd->reply.offset);
 		if (ret < 0)
-			return ret;
+			goto out;
 	}
 
+out:
 	free(msg);
 	free(cmd);
 
-	return 0;
+	if (memfd >= 0)
+		close(memfd);
+
+	return ret < 0 ? ret : 0;
 }
 
 int kdbus_msg_send(const struct kdbus_conn *conn, const char *name,
-- 
1.8.3.1

Re: [PATCH v2 0/5] selftests/kdbus: small fixes

[Shuah Khan]

On 06/17/2015 10:33 AM, Sergei Zviagintsev wrote:
> Set of different fixes over selftests/kdbus.
> 
> These were first sent as independent patches, and in v2 they just
> properly threaded.
> 
> Links to previous patches:
> 
> http://lkml.kernel.org/g/1434550697-24785-1-git-send-email-sergei@s15v.net
> http://lkml.kernel.org/g/1434550697-24785-2-git-send-email-sergei@s15v.net
> http://lkml.kernel.org/g/1434550697-24785-3-git-send-email-sergei@s15v.net
> http://lkml.kernel.org/g/1434550697-24785-4-git-send-email-sergei@s15v.net
> http://lkml.kernel.org/g/1434550697-24785-5-git-send-email-sergei@s15v.net
> 
> Sergei Zviagintsev (5):
>   selftests/kdbus: handle cap_get_proc() error properly
>   selftests/kdbus: drop useless assignment
>   selftests/kdbus: remove useless initializations from
>     kdbus_clone_userns_test()
>   selftests/kdbus: drop duplicated code from __kdbus_msg_send()
>   selftests/kdbus: fix error paths in __kdbus_msg_send()
> 
>  tools/testing/selftests/kdbus/kdbus-util.c       | 46 ++++++++++++------------
>  tools/testing/selftests/kdbus/test-metadata-ns.c |  9 ++---
>  2 files changed, 27 insertions(+), 28 deletions(-)
> 

Nice work! Thanks for these fixes.
Greg! Are you going to apply these into your kdbus tree?

thanks,
-- Shuah

-- 
Shuah Khan
Sr. Linux Kernel Developer
Open Source Innovation Group
Samsung Research America (Silicon Valley)
shuahkh@osg.samsung.com | (970) 217-8978

Re: [PATCH v2 0/5] selftests/kdbus: small fixes

[Greg Kroah-Hartman]

On Wed, Jun 17, 2015 at 11:07:46AM -0600, Shuah Khan wrote:
> On 06/17/2015 10:33 AM, Sergei Zviagintsev wrote:
> > Set of different fixes over selftests/kdbus.
> > 
> > These were first sent as independent patches, and in v2 they just
> > properly threaded.
> > 
> > Links to previous patches:
> > 
> > http://lkml.kernel.org/g/1434550697-24785-1-git-send-email-sergei@s15v.net
> > http://lkml.kernel.org/g/1434550697-24785-2-git-send-email-sergei@s15v.net
> > http://lkml.kernel.org/g/1434550697-24785-3-git-send-email-sergei@s15v.net
> > http://lkml.kernel.org/g/1434550697-24785-4-git-send-email-sergei@s15v.net
> > http://lkml.kernel.org/g/1434550697-24785-5-git-send-email-sergei@s15v.net
> > 
> > Sergei Zviagintsev (5):
> >   selftests/kdbus: handle cap_get_proc() error properly
> >   selftests/kdbus: drop useless assignment
> >   selftests/kdbus: remove useless initializations from
> >     kdbus_clone_userns_test()
> >   selftests/kdbus: drop duplicated code from __kdbus_msg_send()
> >   selftests/kdbus: fix error paths in __kdbus_msg_send()
> > 
> >  tools/testing/selftests/kdbus/kdbus-util.c       | 46 ++++++++++++------------
> >  tools/testing/selftests/kdbus/test-metadata-ns.c |  9 ++---
> >  2 files changed, 27 insertions(+), 28 deletions(-)
> > 
> 
> Nice work! Thanks for these fixes.
> Greg! Are you going to apply these into your kdbus tree?

Yes, I will take them.

thanks,

greg k-h

Re: [PATCH v2 0/5] selftests/kdbus: small fixes

[David Herrmann]

Hi

On Wed, Jun 17, 2015 at 9:11 PM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Wed, Jun 17, 2015 at 11:07:46AM -0600, Shuah Khan wrote:
>> On 06/17/2015 10:33 AM, Sergei Zviagintsev wrote:
>> > Set of different fixes over selftests/kdbus.
>> >
>> > These were first sent as independent patches, and in v2 they just
>> > properly threaded.
>> >
>> > Links to previous patches:
>> >
>> > http://lkml.kernel.org/g/1434550697-24785-1-git-send-email-sergei@s15v.net
>> > http://lkml.kernel.org/g/1434550697-24785-2-git-send-email-sergei@s15v.net
>> > http://lkml.kernel.org/g/1434550697-24785-3-git-send-email-sergei@s15v.net
>> > http://lkml.kernel.org/g/1434550697-24785-4-git-send-email-sergei@s15v.net
>> > http://lkml.kernel.org/g/1434550697-24785-5-git-send-email-sergei@s15v.net
>> >
>> > Sergei Zviagintsev (5):
>> >   selftests/kdbus: handle cap_get_proc() error properly
>> >   selftests/kdbus: drop useless assignment
>> >   selftests/kdbus: remove useless initializations from
>> >     kdbus_clone_userns_test()
>> >   selftests/kdbus: drop duplicated code from __kdbus_msg_send()
>> >   selftests/kdbus: fix error paths in __kdbus_msg_send()
>> >
>> >  tools/testing/selftests/kdbus/kdbus-util.c       | 46 ++++++++++++------------
>> >  tools/testing/selftests/kdbus/test-metadata-ns.c |  9 ++---
>> >  2 files changed, 27 insertions(+), 28 deletions(-)
>> >
>>
>> Nice work! Thanks for these fixes.
>> Greg! Are you going to apply these into your kdbus tree?
>
> Yes, I will take them.

The 5 selftest changes are:

Reviewed-by: David Herrmann <dh.herrmann@gmail.com>

Thanks
David

Re: [PATCH v2 0/5] selftests/kdbus: small fixes

[Greg Kroah-Hartman]

On Wed, Jun 17, 2015 at 07:33:23PM +0300, Sergei Zviagintsev wrote:
> Set of different fixes over selftests/kdbus.
> 
> These were first sent as independent patches, and in v2 they just
> properly threaded.
> 
> Links to previous patches:
> 
> http://lkml.kernel.org/g/1434550697-24785-1-git-send-email-sergei@s15v.net
> http://lkml.kernel.org/g/1434550697-24785-2-git-send-email-sergei@s15v.net
> http://lkml.kernel.org/g/1434550697-24785-3-git-send-email-sergei@s15v.net
> http://lkml.kernel.org/g/1434550697-24785-4-git-send-email-sergei@s15v.net
> http://lkml.kernel.org/g/1434550697-24785-5-git-send-email-sergei@s15v.net
> 
> Sergei Zviagintsev (5):
>   selftests/kdbus: handle cap_get_proc() error properly
>   selftests/kdbus: drop useless assignment
>   selftests/kdbus: remove useless initializations from
>     kdbus_clone_userns_test()
>   selftests/kdbus: drop duplicated code from __kdbus_msg_send()
>   selftests/kdbus: fix error paths in __kdbus_msg_send()
> 
>  tools/testing/selftests/kdbus/kdbus-util.c       | 46 ++++++++++++------------
>  tools/testing/selftests/kdbus/test-metadata-ns.c |  9 ++---
>  2 files changed, 27 insertions(+), 28 deletions(-)

All now applied, thanks.

greg k-h