On (05/08/16 11:56), Pavel Březina wrote: >On 07/27/2016 11:17 AM, Petr Cech wrote: >> 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<pc...@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. > >Ack. master: * dc30c60f166ad9adc63a47a1013508a71624ac87 * e4ba22c165be4b0ccd0e1b9fc92f2f1e35f78a82
LS _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
