2015-06-17 16:33:46

by Sergei Zviagintsev

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

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/[email protected]
http://lkml.kernel.org/g/[email protected]
http://lkml.kernel.org/g/[email protected]
http://lkml.kernel.org/g/[email protected]
http://lkml.kernel.org/g/[email protected]

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


2015-06-17 16:34:49

by Sergei Zviagintsev

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

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

Signed-off-by: Sergei Zviagintsev <[email protected]>
---
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

2015-06-17 16:34:44

by Sergei Zviagintsev

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

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

Signed-off-by: Sergei Zviagintsev <[email protected]>
---
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

2015-06-17 16:34:20

by Sergei Zviagintsev

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

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 <[email protected]>
---
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

2015-06-17 16:33:57

by Sergei Zviagintsev

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

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

Signed-off-by: Sergei Zviagintsev <[email protected]>
---
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

2015-06-17 16:34:08

by Sergei Zviagintsev

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

Handle errors properly, free allocated resources.

Signed-off-by: Sergei Zviagintsev <[email protected]>
---
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

2015-06-17 17:08:09

by Shuah Khan

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

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/[email protected]
> http://lkml.kernel.org/g/[email protected]
> http://lkml.kernel.org/g/[email protected]
> http://lkml.kernel.org/g/[email protected]
> http://lkml.kernel.org/g/[email protected]
>
> 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)
[email protected] | (970) 217-8978

2015-06-17 19:11:44

by Greg Kroah-Hartman

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

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/[email protected]
> > http://lkml.kernel.org/g/[email protected]
> > http://lkml.kernel.org/g/[email protected]
> > http://lkml.kernel.org/g/[email protected]
> > http://lkml.kernel.org/g/[email protected]
> >
> > 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

2015-06-18 10:06:02

by David Herrmann

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

Hi

On Wed, Jun 17, 2015 at 9:11 PM, Greg Kroah-Hartman
<[email protected]> 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/[email protected]
>> > http://lkml.kernel.org/g/[email protected]
>> > http://lkml.kernel.org/g/[email protected]
>> > http://lkml.kernel.org/g/[email protected]
>> > http://lkml.kernel.org/g/[email protected]
>> >
>> > 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 <[email protected]>

Thanks
David

2015-06-19 04:28:03

by Greg Kroah-Hartman

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

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/[email protected]
> http://lkml.kernel.org/g/[email protected]
> http://lkml.kernel.org/g/[email protected]
> http://lkml.kernel.org/g/[email protected]
> http://lkml.kernel.org/g/[email protected]
>
> 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