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<jhro...@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<jhro...@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" \
+                                };
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Reply via email to