1:10 a.m.
Generate DBus1-like error messages on kernel error notifications.
---
ell/dbus-kernel.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/ell/dbus-kernel.c b/ell/dbus-kernel.c
index b5a07a5..4fd2f29 100644
--- a/ell/dbus-kernel.c
+++ b/ell/dbus-kernel.c
@@ -559,6 +559,7 @@ int _dbus_kernel_recv(int fd, void *kdbus_pool,
struct kdbus_item *item;
int r;
size_t min_size;
+ const char *error;
memset(&recv_cmd, 0, sizeof(recv_cmd));
@@ -601,6 +602,23 @@ int _dbus_kernel_recv(int fd, void *kdbus_pool,
user_data);
break;
+ case KDBUS_ITEM_REPLY_TIMEOUT:
+ case KDBUS_ITEM_REPLY_DEAD:
+ if (item->type == KDBUS_ITEM_REPLY_TIMEOUT)
+ error = "Did not receive a reply.";
+ else
+ error = "Message recipient disconnected from "
+ "message bus without replying.";
+
+ dbus_msg = _dbus_message_new_error(
+ 2, msg->cookie_reply, NULL,
+ "org.freedesktop.DBus.Error.NoReply",
+ error);
+ if (dbus_msg)
+ message_func(dbus_msg, user_data);
+
+ break;
+
default:
break;
}
--
2.5.0
1:10 a.m.
New subject: [PATCH 1/5] gvariant: _gvariant_num_children error check
---
ell/gvariant-util.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/ell/gvariant-util.c b/ell/gvariant-util.c
index 88bb74e..17606f8 100644
--- a/ell/gvariant-util.c
+++ b/ell/gvariant-util.c
@@ -374,7 +374,7 @@ static bool gvariant_iter_init_internal(struct l_dbus_message_iter
*iter,
unsigned int alignment : 4;
size_t end; /* Index past the end of the type */
} *children;
- uint8_t n_children;
+ int n_children;
if (sig_end) {
size_t len = sig_end - sig_start;
@@ -392,6 +392,9 @@ static bool gvariant_iter_init_internal(struct l_dbus_message_iter
*iter,
iter->pos = 0;
n_children = _gvariant_num_children(subsig);
+ if (n_children < 0)
+ return false;
+
children = l_new(struct gvariant_type_info, n_children);
for (p = sig_start, i = 0; i < n_children; i++) {
--
2.5.0
1:10 a.m.
New subject: [PATCH 2/5] gvariant: Allow empty signature in enter_struct_dict_common
gvariant_iter_init_internal now checks if the signature is empty before
calling _gvariant_num_children which would return an error in that case.
The gvariant_iter_init_internal caller is responsible for checking if
the signature is allowed in the specific context.
---
ell/gvariant-util.c | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/ell/gvariant-util.c b/ell/gvariant-util.c
index 17606f8..1f493e0 100644
--- a/ell/gvariant-util.c
+++ b/ell/gvariant-util.c
@@ -391,11 +391,17 @@ static bool gvariant_iter_init_internal(struct l_dbus_message_iter
*iter,
iter->len = len;
iter->pos = 0;
- n_children = _gvariant_num_children(subsig);
- if (n_children < 0)
- return false;
+ if (subsig[0] != '\0') {
+ n_children = _gvariant_num_children(subsig);
+ if (n_children < 0)
+ return false;
- children = l_new(struct gvariant_type_info, n_children);
+ children = l_new(struct gvariant_type_info, n_children);
+ } else {
+ n_children = 0;
+
+ children = NULL;
+ }
for (p = sig_start, i = 0; i < n_children; i++) {
int alignment;
@@ -966,7 +972,7 @@ static bool enter_struct_dict_common(struct dbus_builder *builder,
bool _gvariant_builder_enter_struct(struct dbus_builder *builder,
const char *signature)
{
- if (!_gvariant_valid_signature(signature))
+ if (signature[0] && !_gvariant_valid_signature(signature))
return false;
return enter_struct_dict_common(builder, signature,
--
2.5.0
4:01 a.m.
New subject: [PATCH 2/5] gvariant: Allow empty signature in enter_struct_dict_common
Hi Andrew,
On 04/22/2016 08:10 PM, Andrew Zaborowski wrote:
gvariant_iter_init_internal now checks if the signature is empty
before
calling _gvariant_num_children which would return an error in that case.
The gvariant_iter_init_internal caller is responsible for checking if
the signature is allowed in the specific context.
---
ell/gvariant-util.c | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
I went ahead an applied this one. But one question, what is the
practical utility of allowing iterators into the empty struct?
Regards,
-Denis
9:08 a.m.
New subject: [PATCH 2/5] gvariant: Allow empty signature in enter_struct_dict_common
Hi Denis,
On 26 April 2016 at 06:01, Denis Kenzior <denkenz(a)gmail.com> wrote:
Hi Andrew,
On 04/22/2016 08:10 PM, Andrew Zaborowski wrote:
>
> gvariant_iter_init_internal now checks if the signature is empty before
> calling _gvariant_num_children which would return an error in that case.
> The gvariant_iter_init_internal caller is responsible for checking if
> the signature is allowed in the specific context.
> ---
> ell/gvariant-util.c | 16 +++++++++++-----
> 1 file changed, 11 insertions(+), 5 deletions(-)
>
I went ahead an applied this one. But one question, what is the practical
Thanks
utility of allowing iterators into the empty struct?
Well internally in ell we have a clear use case when we parse kdbus
messages, we use this to get the body start, body size and signature.
I also suspect the public function l_dbus_message_get_arguments makes
use of that when we're calling it with the empty signature to check
that the message signature is empty as expected.
Best regards
1:10 a.m.
New subject: [PATCH 3/5] gvariant: Make sure builder outputs a zero for empty struct
Make sure the single byte being written by the builder is a zero.
I had assumed grow_body would zero the newly allocated body bytes but on
a closer look it zeros the alignment padding only.
---
ell/gvariant-util.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/ell/gvariant-util.c b/ell/gvariant-util.c
index 1f493e0..2172ad9 100644
--- a/ell/gvariant-util.c
+++ b/ell/gvariant-util.c
@@ -1016,9 +1016,12 @@ static bool leave_struct_dict_common(struct dbus_builder *builder,
int alignment = _gvariant_get_alignment(container->signature);
grow_body(builder, 0, alignment);
- /* Empty struct or "unit type" is a encoded as a zero byte */
- if (container->signature[0] == '\0')
- grow_body(builder, 1, 1);
+ /* Empty struct or "unit type" is encoded as a zero byte */
+ if (container->signature[0] == '\0') {
+ size_t start = grow_body(builder, 1, 1);
+
+ memset(builder->body + start, 0, 1);
+ }
parent->variable_is_last = false;
} else {
@@ -1323,9 +1326,12 @@ char *_gvariant_builder_finish(struct dbus_builder *builder,
int alignment = _gvariant_get_alignment(signature);
grow_body(builder, 0, alignment);
- /* Empty struct or "unit type" is encoded one zero byte */
- if (signature[0] == '\0')
- grow_body(builder, 1, 1);
+ /* Empty struct or "unit type" is encoded as a zero byte */
+ if (signature[0] == '\0') {
+ size_t start = grow_body(builder, 1, 1);
+
+ memset(builder->body + start, 0, 1);
+ }
} else
container_append_struct_offsets(root, builder);
--
2.5.0
4:01 a.m.
New subject: [PATCH 3/5] gvariant: Make sure builder outputs a zero for empty struct
Hi Andrew,
On 04/22/2016 08:10 PM, Andrew Zaborowski wrote:
Make sure the single byte being written by the builder is a zero.
I had assumed grow_body would zero the newly allocated body bytes but on
a closer look it zeros the alignment padding only.
---
ell/gvariant-util.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
Applied, thanks.
Regards,
-Denis
1:10 a.m.
New subject: [PATCH 4/5] unit: Gvariant iterator and builder test for empty structs
---
unit/test-gvariant-util.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 51 insertions(+)
diff --git a/unit/test-gvariant-util.c b/unit/test-gvariant-util.c
index 9714df0..331b1f7 100644
--- a/unit/test-gvariant-util.c
+++ b/unit/test-gvariant-util.c
@@ -553,6 +553,32 @@ static void test_iter_nested_struct_1(const void *test_data)
assert(i == -1);
}
+static const unsigned char empty_struct_data_1[] = {
+ 0x00
+};
+
+static struct parser_data empty_struct_1 = {
+ .data = empty_struct_data_1,
+ .len = sizeof(empty_struct_data_1),
+ .signature = "()",
+};
+
+static void test_iter_empty_struct_1(const void *test_data)
+{
+ const struct parser_data *test = test_data;
+ struct l_dbus_message_iter iter;
+ bool ret;
+ struct l_dbus_message_iter str;
+
+ _gvariant_iter_init(&iter, NULL, test->signature,
+ test->signature + strlen(test->signature),
+ test->data, test->len);
+
+ ret = _gvariant_iter_enter_struct(&iter, &str);
+ assert(ret);
+ assert(str.sig_len == 0);
+}
+
static const unsigned char variant_data_1[] = {
0x05, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x66, 0x6f, 0x6f, 0x62,
0x61, 0x72, 0x00, 0x00, 0x73, 0x00, 0x00, 0x00, 0x05, 0x00, 0x00, 0x00,
@@ -1360,6 +1386,25 @@ static void test_builder_nested_struct_1(const void *test_data)
FINISH_AND_CHECK_BUILT_RESULT();
}
+static void test_builder_empty_struct_1(const void *test_data)
+{
+ const struct parser_data *test = test_data;
+ struct dbus_builder *builder;
+ bool ret;
+ BUILDER_TEST_HEADER();
+
+ builder = _gvariant_builder_new(NULL, 0);
+ assert(builder);
+
+ ret = _gvariant_builder_enter_struct(builder, "");
+ assert(ret);
+
+ ret = _gvariant_builder_leave_struct(builder);
+ assert(ret);
+
+ FINISH_AND_CHECK_BUILT_RESULT();
+}
+
static void test_builder_variant_1(const void *test_data)
{
const struct parser_data *test = test_data;
@@ -1840,6 +1885,9 @@ int main(int argc, char *argv[])
l_test_add("Iter Test Nested Struct '((us)yi)'",
test_iter_nested_struct_1, &nested_struct_1);
+ l_test_add("Iter Test Empty Struct '()'",
+ test_iter_empty_struct_1, &empty_struct_1);
+
l_test_add("Iter Test Variant '(uvu)i'", test_iter_variant_1,
&variant_1);
l_test_add("Iter Test Variant 'v'", test_iter_variant_2,
&variant_2);
@@ -1876,6 +1924,9 @@ int main(int argc, char *argv[])
l_test_add("Builder Test Nested Struct '((us)yi)'",
test_builder_nested_struct_1, &nested_struct_1);
+ l_test_add("Builder Test Empty Struct '()'",
+ test_builder_empty_struct_1, &empty_struct_1);
+
l_test_add("Builder Test Variant '(uvu)i'", test_builder_variant_1,
&variant_1);
l_test_add("Builder Test Variant 'v'", test_builder_variant_2,
--
2.5.0
4:01 a.m.
New subject: [PATCH 4/5] unit: Gvariant iterator and builder test for empty structs
Hi Andrew,
On 04/22/2016 08:10 PM, Andrew Zaborowski wrote:
---
unit/test-gvariant-util.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 51 insertions(+)
Applied, thanks.
Regards,
-Denis
1:10 a.m.
New subject: [PATCH 5/5] unit: Gvariant method call with no arguments tests
---
unit/test-gvariant-message.c | 51 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 51 insertions(+)
diff --git a/unit/test-gvariant-message.c b/unit/test-gvariant-message.c
index 836ba29..85326a6 100644
--- a/unit/test-gvariant-message.c
+++ b/unit/test-gvariant-message.c
@@ -110,6 +110,30 @@ static const struct message_data message_data_complex_1 = {
.binary_len = sizeof(message_binary_complex_1),
};
+static const unsigned char message_binary_empty_sig[] = {
+ 0x6c, 0x01, 0x00, 0x02, 0x00, 0x00, 0x00, 0x00, 0x57, 0x04, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x2f, 0x66, 0x6f, 0x6f, 0x2f, 0x62, 0x61, 0x72, 0x00, 0x00, 0x6f, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x03, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x46, 0x6f, 0x6f, 0x62, 0x61, 0x72, 0x00, 0x00, 0x73, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x66, 0x6f, 0x6f, 0x2e, 0x62, 0x61, 0x72, 0x00, 0x00, 0x73, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x06, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x66, 0x6f, 0x6f, 0x2e, 0x62, 0x61, 0x72, 0x00, 0x00, 0x73, 0x13, 0x29,
+ 0x42, 0x5a, 0x00, 0x00, 0x00, 0x00, 0x28, 0x29, 0x6e,
+};
+
+static const struct message_data message_data_empty_sig = {
+ .type = "method_call",
+ .path = "/foo/bar",
+ .interface = "foo.bar",
+ .member = "Foobar",
+ .destination = "foo.bar",
+ .signature = "",
+ .binary = message_binary_empty_sig,
+ .binary_len = sizeof(message_binary_empty_sig),
+};
+
static struct l_dbus_message *check_message(const struct message_data *msg_data)
{
struct l_dbus_message *msg;
@@ -303,6 +327,28 @@ static void build_complex_1(const void *data)
compare_message(msg, data);
}
+static void check_empty_sig(const void *data)
+{
+ struct l_dbus_message *msg = check_message(data);
+
+ assert(l_dbus_message_get_arguments(msg, ""));
+
+ l_dbus_message_unref(msg);
+}
+
+static void build_empty_sig(const void *data)
+{
+ struct l_dbus_message *msg = build_message(data);
+ bool result;
+
+ result = l_dbus_message_set_arguments(msg, "");
+ assert(result);
+
+ _dbus_message_set_serial(msg, 1111);
+
+ compare_message(msg, data);
+}
+
static void builder_rewind(const void *data)
{
struct l_dbus_message *msg = build_message(data);
@@ -364,6 +410,11 @@ int main(int argc, char *argv[])
l_test_add("Complex 1 (build)", build_complex_1,
&message_data_complex_1);
+ l_test_add("Empty signature (parse)", check_empty_sig,
+ &message_data_empty_sig);
+ l_test_add("Empty signature (build)", build_empty_sig,
+ &message_data_empty_sig);
+
l_test_add("Message Builder Rewind Complex 1", builder_rewind,
&message_data_complex_1);
--
2.5.0
4:02 a.m.
New subject: [PATCH 5/5] unit: Gvariant method call with no arguments tests
Hi Andrew,
On 04/22/2016 08:10 PM, Andrew Zaborowski wrote:
---
unit/test-gvariant-message.c | 51 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 51 insertions(+)
Applied, thanks.
Regards,
-Denis
1770
days inactive
1773
days old
12 comments
3 participants
participants (3)
-
Andrew Zaborowski -
Andrzej Zaborowski
-
Denis Kenzior