[SSSD] [PATCH] sss_sifp: set output parameters if attribute is NULL

Lukas Slebodnik lslebodn at redhat.com
Wed Jul 16 12:10:16 UTC 2014


On (16/07/14 13:46), Pavel Březina wrote:
>On 07/16/2014 01:37 PM, Pavel Březina wrote:
>>On 07/10/2014 02:08 PM, Lukas Slebodnik wrote:
>>>On (02/07/14 13:09), Pavel Březina wrote:
>>>>First patch is a minor bug in unit test I found when I was writing
>>>>new tests.
>>>>The rest is described in commit message.
>>>
>>>>From de3ed7bf0e9784058241e4b532c72e324e3dd635 Mon Sep 17 00:00:00 2001
>>>>From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina at redhat.com>
>>>>Date: Tue, 1 Jul 2014 11:55:41 +0200
>>>>Subject: [PATCH 2/4] sss_sifp: set output parameters if attribute is
>>>>NULL
>>>>
>>>>There are two cases that may happen when a user calls Get or GetAll:
>>>>1) the attribute is missing
>>>>2) the attribute is empty
>>>>
>>>>sss_sifp has two error code to distinguish between those two cases:
>>>>1) SSS_SIFP_ATTR_MISSING
>>>>2) SSS_SIFP_ATTR_NULL
>>>>
>>>>Usually the caller is not interested on situations when the attribute
>>>>is empty and it can be considered as error. Having it as a separate
>>>>error code instead of setting the output value to NULL is necesarry
>>>>since attribute does not have to be a pointer.
>>>>
>>>>This patch however sets pointer type attributes to NULL since it may
>>>>simplify the code path when the caller is actually interested in
>>>>this information (e. g. empty server list on domain objects).
>>>>
>>>>It is not possible to send a NULL string over a D-Bus nor it is
>>>>possible to have hash table NULL with current code so these two
>>>>scenarios are not tested. However, it is handled in sss_sifp_attr
>>>>code for completeness.
>>>>---
>>>>src/lib/sifp/sss_sifp_attrs.c    |  63 +++++---
>>>>src/tests/cmocka/test_sss_sifp.c | 338
>>>>+++++++++++++++++++++++++++++++++++++++
>>>>2 files changed, 380 insertions(+), 21 deletions(-)
>>>>
>>>>diff --git a/src/lib/sifp/sss_sifp_attrs.c
>>>>b/src/lib/sifp/sss_sifp_attrs.c
>>>>index
>>>>6d10c46119989b42e79cc80f251fa275d07b1cd0..5a0241f85f734891f5273d1f04e5721be859b263
>>>>100644
>>>>--- a/src/lib/sifp/sss_sifp_attrs.c
>>>>+++ b/src/lib/sifp/sss_sifp_attrs.c
>>>>@@ -41,23 +41,31 @@
>>>>     out =
>>>>attr->data.field[0];                                              \
>>>>} while (0)
>>>>
>>>>-#define GET_ATTR_ARRAY(attrs, name, rtype, field, out_num, out_val)
>>>>do {    \
>>>>+#define GET_ATTR_ARRAY(attrs, name, rtype, field, out_num, out_val,
>>>>ret)    \
>>>>+do
>>>>{
>>>>\
>>>>     sss_sifp_attr *attr = sss_sifp_find_attr(attrs,
>>>>name);                  \
>>>>
>>>>\
>>>>     if (attr == NULL)
>>>>{                                                     \
>>>>-        return
>>>>SSS_SIFP_ATTR_MISSING;                                       \
>>>>+        ret =
>>>>SSS_SIFP_ATTR_MISSING;                                        \
>>>>+
>>>>break;                                                              \
>>>>
>>>>}
>>>>\
>>>>
>>>>\
>>>>     if (attr->type != rtype)
>>>>{                                              \
>>>>-        return
>>>>SSS_SIFP_INCORRECT_TYPE;                                     \
>>>>+        ret =
>>>>SSS_SIFP_INCORRECT_TYPE;                                      \
>>>>+
>>>>break;                                                              \
>>>>
>>>>}
>>>>\
>>>>
>>>>\
>>>>     if (attr->data.field == NULL)
>>>>{                                         \
>>>>-        return
>>>>SSS_SIFP_ATTR_NULL;                                          \
>>>>+        out_num =
>>>>0;                                                        \
>>>>+        out_val =
>>>>NULL;                                                     \
>>>>+        ret =
>>>>SSS_SIFP_ATTR_NULL;                                           \
>>>>+
>>>>break;                                                              \
>>>>
>>>>}
>>>>\
>>>>
>>>>\
>>>>     out_num =
>>>>attr->num_values;                                             \
>>>>     out_val =
>>>>attr->data.field;                                             \
>>>>+
>>>>\
>>>>+    ret =
>>>>SSS_SIFP_OK;                                                      \
>>>>} while (0)
>>>>
>>>>@@ -151,6 +159,7 @@ sss_sifp_find_attr_as_string(sss_sifp_attr **attrs,
>>>>     GET_ATTR(attrs, name, SSS_SIFP_ATTR_TYPE_STRING, str, value);
>>>>
>>>
>>>function sss_sifp_find_attr_as_string can return SSS_SIFP_ATTR_NULL
>>>also from code generated by macro GET_ATTR.
>>>Do we want to set value to NULL there?
>>>
>>>Other wise patches looks good.
>>>
>>>LS
>>
>>Thanks, fixed.
>
>Sorry, I did not commit the changes. So one more time.
>
>Is there any way to make git format-patch to warn if there are unstaged
>changes?
>

Thank you.

ACK to all

LS



More information about the sssd-devel mailing list