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(a)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.