[SSSD] [PATCH] Reduce the code duplication in Data Provider

Petr Cech pcech at redhat.com
Mon Nov 16 07:12:06 UTC 2015


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 at 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 at 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" \
> +                                };


More information about the sssd-devel mailing list