Hi,
the second patch fixes https://fedorahosted.org/sssd/ticket/1630 while the first adds a missing talloc_free(). One might argue that both talloc_free() can be removed because the parent context is freed later during the request.
bye, Sumit
On Fri, Aug 16, 2013 at 06:35:52PM +0200, Sumit Bose wrote:
Hi,
the second patch fixes https://fedorahosted.org/sssd/ticket/1630 while the first adds a missing talloc_free(). One might argue that both talloc_free() can be removed because the parent context is freed later during the request.
bye, Sumit
From 8af12d60f892d25808a1a74517b9c2c2ff40eadf Mon Sep 17 00:00:00 2001 From: Sumit Bose sbose@redhat.com Date: Tue, 13 Aug 2013 17:59:11 +0200 Subject: [PATCH 1/2] ipa_s2n_get_user_done: free group_attrs as well
src/providers/ipa/ipa_s2n_exop.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/src/providers/ipa/ipa_s2n_exop.c b/src/providers/ipa/ipa_s2n_exop.c index e16ec14..53746ca 100644 --- a/src/providers/ipa/ipa_s2n_exop.c +++ b/src/providers/ipa/ipa_s2n_exop.c @@ -882,6 +882,7 @@ static void ipa_s2n_get_user_done(struct tevent_req *subreq)
done: talloc_free(user_attrs);
- talloc_free(group_attrs); if (ret == EOK) { tevent_req_done(req); } else {
-- 1.7.7.6
ACK. I think if the request is not expected to be a long-running one (like a nested group request) we can get away with simply using state.
From 7c54aef6677a8498907d57156f661992d91055e9 Mon Sep 17 00:00:00 2001 From: Sumit Bose sbose@redhat.com Date: Tue, 13 Aug 2013 17:59:42 +0200 Subject: [PATCH 2/2] ipa_s2n_get_user_done: make sure ALIAS name is lower case
Fixes https://fedorahosted.org/sssd/ticket/1630
src/providers/ipa/ipa_s2n_exop.c | 20 ++++++++++++++++++-- 1 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/src/providers/ipa/ipa_s2n_exop.c b/src/providers/ipa/ipa_s2n_exop.c index 53746ca..d8506aa 100644 --- a/src/providers/ipa/ipa_s2n_exop.c +++ b/src/providers/ipa/ipa_s2n_exop.c @@ -651,6 +651,7 @@ static void ipa_s2n_get_user_done(struct tevent_req *subreq) struct sysdb_attrs *user_attrs = NULL; struct sysdb_attrs *group_attrs = NULL; char *name;
- char *lc_name; char *realm; char *upn; struct berval *bv_req = NULL;
@@ -766,7 +767,14 @@ static void ipa_s2n_get_user_done(struct tevent_req *subreq) goto done; }
ret = sysdb_attrs_add_string(user_attrs, SYSDB_NAME_ALIAS, name);
lc_name = sss_tc_utf8_str_tolower(user_attrs, name);if (lc_name == NULL) {DEBUG(SSSDBG_CRIT_FAILURE, ("Cannot convert name to lowercase\n"));ret = ENOMEM;goto done;}ret = sysdb_attrs_add_string(user_attrs, SYSDB_NAME_ALIAS, lc_name); if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, ("sysdb_attrs_add_string failed.\n")); goto done;@@ -844,8 +852,16 @@ static void ipa_s2n_get_user_done(struct tevent_req *subreq) goto done; }
lc_name = sss_tc_utf8_str_tolower(group_attrs, name);if (lc_name == NULL) {DEBUG(SSSDBG_CRIT_FAILURE,("Cannot convert name to lowercase\n"));ret = ENOMEM;goto done;}ret = sysdb_attrs_add_string(group_attrs, SYSDB_NAME_ALIAS,
name);
lc_name); if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, ("sysdb_attrs_add_string failed.\n")); goto done;-- 1.7.7.6
ACK. I haven't been able to reproduce the issue because in my configuration (F-19) the name I got from extop was already lowercased. I think the bug depended on some version of Winbind? Anyway, the code works fine, the nameAlias attribute is populated and is lowercased.
On Mon, Aug 19, 2013 at 12:28:57PM +0200, Jakub Hrozek wrote:
On Fri, Aug 16, 2013 at 06:35:52PM +0200, Sumit Bose wrote:
Hi,
the second patch fixes https://fedorahosted.org/sssd/ticket/1630 while the first adds a missing talloc_free(). One might argue that both talloc_free() can be removed because the parent context is freed later during the request.
bye, Sumit
From 8af12d60f892d25808a1a74517b9c2c2ff40eadf Mon Sep 17 00:00:00 2001 From: Sumit Bose sbose@redhat.com Date: Tue, 13 Aug 2013 17:59:11 +0200 Subject: [PATCH 1/2] ipa_s2n_get_user_done: free group_attrs as well
src/providers/ipa/ipa_s2n_exop.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/src/providers/ipa/ipa_s2n_exop.c b/src/providers/ipa/ipa_s2n_exop.c index e16ec14..53746ca 100644 --- a/src/providers/ipa/ipa_s2n_exop.c +++ b/src/providers/ipa/ipa_s2n_exop.c @@ -882,6 +882,7 @@ static void ipa_s2n_get_user_done(struct tevent_req *subreq)
done: talloc_free(user_attrs);
- talloc_free(group_attrs); if (ret == EOK) { tevent_req_done(req); } else {
-- 1.7.7.6
ACK. I think if the request is not expected to be a long-running one (like a nested group request) we can get away with simply using state.
From 7c54aef6677a8498907d57156f661992d91055e9 Mon Sep 17 00:00:00 2001 From: Sumit Bose sbose@redhat.com Date: Tue, 13 Aug 2013 17:59:42 +0200 Subject: [PATCH 2/2] ipa_s2n_get_user_done: make sure ALIAS name is lower case
Fixes https://fedorahosted.org/sssd/ticket/1630
src/providers/ipa/ipa_s2n_exop.c | 20 ++++++++++++++++++-- 1 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/src/providers/ipa/ipa_s2n_exop.c b/src/providers/ipa/ipa_s2n_exop.c index 53746ca..d8506aa 100644 --- a/src/providers/ipa/ipa_s2n_exop.c +++ b/src/providers/ipa/ipa_s2n_exop.c @@ -651,6 +651,7 @@ static void ipa_s2n_get_user_done(struct tevent_req *subreq) struct sysdb_attrs *user_attrs = NULL; struct sysdb_attrs *group_attrs = NULL; char *name;
- char *lc_name; char *realm; char *upn; struct berval *bv_req = NULL;
@@ -766,7 +767,14 @@ static void ipa_s2n_get_user_done(struct tevent_req *subreq) goto done; }
ret = sysdb_attrs_add_string(user_attrs, SYSDB_NAME_ALIAS, name);
lc_name = sss_tc_utf8_str_tolower(user_attrs, name);if (lc_name == NULL) {DEBUG(SSSDBG_CRIT_FAILURE, ("Cannot convert name to lowercase\n"));ret = ENOMEM;goto done;}ret = sysdb_attrs_add_string(user_attrs, SYSDB_NAME_ALIAS, lc_name); if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, ("sysdb_attrs_add_string failed.\n")); goto done;@@ -844,8 +852,16 @@ static void ipa_s2n_get_user_done(struct tevent_req *subreq) goto done; }
lc_name = sss_tc_utf8_str_tolower(group_attrs, name);if (lc_name == NULL) {DEBUG(SSSDBG_CRIT_FAILURE,("Cannot convert name to lowercase\n"));ret = ENOMEM;goto done;}ret = sysdb_attrs_add_string(group_attrs, SYSDB_NAME_ALIAS,
name);
lc_name); if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, ("sysdb_attrs_add_string failed.\n")); goto done;-- 1.7.7.6
ACK. I haven't been able to reproduce the issue because in my configuration (F-19) the name I got from extop was already lowercased. I think the bug depended on some version of Winbind? Anyway, the code works fine, the nameAlias attribute is populated and is lowercased.
Pushed both to master and sssd-1-10
sssd-devel@lists.fedorahosted.org
-
Jakub Hrozek -
Sumit Bose