On 11/11/2015 02:28 PM, Jakub Hrozek wrote:
Hi,
I think one of the prime reasons for #2861 was copy-pasting code. The
two attached patches reduce the code duplication and hopefully will make
future additions to Data Provider safer.
Ideas on different solutions are welcome!
Hello Jakub,
I see that the previous thread is pushed. So I have started to do review
of those patch. Unfortunately the CI tests environment seems to be
broken at all, however, local tests passed.
Anyway, I have one little question, look to the second patch.
0001-DP-Reduce-code-duplication-in-the-callback-handlers.patch
From f6e929d4132a23d23a9016e43f4b870780c53032 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek<jhrozek(a)redhat.com>
Date: Wed, 11 Nov 2015 13:39:43 +0100
Subject: [PATCH 1/2] DP: Reduce code duplication in the callback handlers
Instead of calling sbus_request_return_and_finish() directly with the
same checks copied over, add a be_sbus_reply() helper instead.
---
[...]
0002-DP-Reduce-code-duplication-in-Data-Provider-handlers.patch
From caeee4a21bda233f0ec8b08b87a0695029e9af8f Mon Sep 17 00:00:00 2001
From: Jakub Hrozek<jhrozek(a)redhat.com>
Date: Wed, 11 Nov 2015 13:40:16 +0100
Subject: [PATCH 2/2] DP: Reduce code duplication in Data Provider handlers
Instead of setting the three same variables over again, add a structure
be_sbus_reply_data with a default initializer BE_SBUS_REPLY_DATA_INIT.
The handlers can then set the structure to BE_SBUS_REPLY_DATA_INIT on
declaration or set a particular value with be_sbus_reply_data_set.
The handler can also reply to the message (typically on failure state)
with be_sbus_req_reply_data()
---
src/providers/data_provider_be.c | 266 +++++++++++++++------------------------
1 file changed, 98 insertions(+), 168 deletions(-)
diff --git a/src/providers/data_provider_be.c b/src/providers/data_provider_be.c
index eb2f49adce5f5313f31c67b1dfdd21685e69ca3a..de8a8357b8230eddb7f49fff021957c3f580c64e
100644
--- a/src/providers/data_provider_be.c
+++ b/src/providers/data_provider_be.c
@@ -319,6 +319,36 @@ static errno_t be_offline_reply(struct sbus_request **sbus_req_ptr,
return ret;
}
+struct be_sbus_reply_data {
+ dbus_uint16_t err_maj;
+ dbus_uint32_t err_min;
+ const char *err_msg;
+};
+
+#define BE_SBUS_REPLY_DATA_INIT { .err_maj = DP_ERR_FATAL, \
--^--
What does this dot means? It is first time that I see it. Could you
explain it to me, please? Is it some kind of syntactic sugar?
Regards
Petr
+ .err_min = EFAULT, \
+ .err_msg = "Fatal error" \
+ };