On 07/27/2016 10:57 AM, Pavel Březina wrote:
On 07/27/2016 10:26 AM, Petr Cech wrote:
> Hi list,
>
> there is patch for [1] attached.
>
> [1]
https://fedorahosted.org/sssd/ticket/3084
>
> If you prefer two commits--one for reverting and second with the
> patch--don't hesitate to tell me it. :-)
>
> Regards
>
> --
> Petr^4 Čech
>
> 0001-PROVIDER-Conversion-empty-string-from-D-Bus-to-NULL.patch
>
>
> From d3942580b3eb1d48a761a91b4c9c68c15cfb8c33 Mon Sep 17 00:00:00 2001
> From: Petr Cech<pcech(a)redhat.com>
> Date: Wed, 27 Jul 2016 09:38:25 +0200
> Subject: [PATCH] PROVIDER: Conversion empty string from D-Bus to NULL
>
> This patch fixes the issue with empty string recieving from D-Bus.
> Data providers obtains NULL. So this is simple conversin.
>
> It also revert "LDAP: Lookup services by all protocols unless
> a protocol is specified". This reverts
> commit aa58e216c1f794bd335151f19e79adbb3ddf4c73.
>
> Resolves:
>
https://fedorahosted.org/sssd/ticket/3084
> ---
> src/providers/data_provider/dp_target_id.c | 4 ++--
> src/providers/ldap/ldap_id_services.c | 7 ++-----
> 2 files changed, 4 insertions(+), 7 deletions(-)
>
> diff --git a/src/providers/data_provider/dp_target_id.c
> b/src/providers/data_provider/dp_target_id.c
> index
> f24b2ccb62c07eb900b5c057a1a3dbf824c553a9..593e2e83e43d9c5ff24a63597fe5bf658252e5c2
> 100644
> --- a/src/providers/data_provider/dp_target_id.c
> +++ b/src/providers/data_provider/dp_target_id.c
> @@ -62,7 +62,7 @@ static bool check_and_parse_filter(struct dp_id_data
> *data,
> {0, 0, 0}};
> int i;
>
> - if (filter == NULL) {
> + if (filter == NULL && filter[0] != '\0') {
SBUS_IS_STRING_EMPTY is a better choice IMHO.
Addressed.
> return false;
> }
>
> @@ -70,7 +70,7 @@ static bool check_and_parse_filter(struct dp_id_data
> *data,
> if (strncmp(filter, types[i].name, types[i].lenght) == 0) {
> data->filter_type = types[i].type;
> data->filter_value = &filter[types[i].lenght];
I think we should call SBUS_SET_STRING even in the line above. I know
filter can't be NULL but in theory you can pass filter="key=" with empty
value and than data->filter_value would be an empty string instead of
NULL. I don't think we use an empty value in filter anywhere though.
Addressed.
> - data->extra_value = extra;
> + data->extra_value = SBUS_SET_STRING(extra);
> return true;
> }
> }
> diff --git a/src/providers/ldap/ldap_id_services.c
> b/src/providers/ldap/ldap_id_services.c
> index
> 401228c20af31ae2df9bb3d35ed25fb6f06b1839..77215127b53297d840eaa4d2f35a75eedb085e43
> 100644
> --- a/src/providers/ldap/ldap_id_services.c
> +++ b/src/providers/ldap/ldap_id_services.c
> @@ -89,9 +89,6 @@ services_get_send(TALLOC_CTX *mem_ctx,
> state->sysdb = sdom->dom->sysdb;
> state->name = name;
> state->protocol = protocol;
> - if (state->protocol != NULL && state->protocol[0] == '\0')
{
> - state->protocol = NULL;
> - }
> state->filter_type = filter_type;
> state->noexist_delete = noexist_delete;
>
> @@ -117,8 +114,8 @@ services_get_send(TALLOC_CTX *mem_ctx,
> ret = sss_filter_sanitize(state, name, &clean_name);
> if (ret != EOK) goto error;
>
> - if (state->protocol != NULL) {
> - ret = sss_filter_sanitize(state, state->protocol,
> &clean_protocol);
> + if (protocol) {
Please use if (protocol == NULL)
Addressed.
This last is changing original code before commit reverting.
Reverted commit used the opposite condition.
Is it right this way?
> + ret = sss_filter_sanitize(state, protocol,
&clean_protocol);
> if (ret != EOK) goto error;
> }
I checked other dp targets and this place is the only one that needs
changing.
Thank you, Jakub and Pavel, for comments.
Fixed patch set is attached.
--
Petr^4 Čech