On (27/05/13 20:37), Jakub Hrozek wrote:
Hi,
I fond these small issues while working on the GC lookups. I'm sending
them separately b/c they can be reviewed without any knowledge about GC
and do not break any existing functionality.
[PATCH 1/4] IPA: Check for ENOMEM
Just an allocation check.
[PATCH 2/4] Remove unneeded comment
Not needed anymore I think
[PATCH 3/4] FO: Fix setting status of duplicate
The fo_set_port_status() function was performing its own duplicate
detection which is wrong because it duplicates existing code and doesn't
take the private user_data structure into account.
[PATCH 4/4] AD dyndns: extract the host name from URI
Because the AD provider might connect to LDAP on different ports
depending on whether it is a Global Catalog or pure LDAP, we need to
extract the hostname using LDAP function calls, not just grab the server
name part from the URI.
From 151dbd2db82e46369ce8c696a2ab6424683bafc1 Mon Sep 17 00:00:00
2001
From: Jakub Hrozek <jhrozek(a)redhat.com>
Date: Wed, 22 May 2013 17:03:33 +0200
Subject: [PATCH 1/4] IPA: Check for ENOMEM
---
src/providers/ipa/ipa_subdomains.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/src/providers/ipa/ipa_subdomains.c b/src/providers/ipa/ipa_subdomains.c
index 98fc69f14d62d74fb82b0f067307362e819aa82c..9c8a926c177ce751466b21b9043920580f70af3c
100644
--- a/src/providers/ipa/ipa_subdomains.c
+++ b/src/providers/ipa/ipa_subdomains.c
@@ -216,6 +216,10 @@ static errno_t ipa_subdom_store(struct sss_domain_info *domain,
int ret;
tmp_ctx = talloc_new(domain);
+ if (tmp_ctx == NULL) {
+ ret = ENOMEM;
+ goto done;
We should immediately fail, if tmp_ctx could not be created.
Next two lines of code are usually used.
DEBUG(SSSDBG_OP_FAILURE, ("talloc_new failed.\n"));
return ENOMEM;
+ }
ret = sysdb_attrs_get_string(attrs, IPA_CN, &name);
if (ret != EOK) {
--
1.8.2.1
From a6cf1dbee6a5ebf31936a47d6d96804f74db282d Mon Sep 17 00:00:00
2001
From: Jakub Hrozek <jhrozek(a)redhat.com>
Date: Sun, 19 May 2013 21:22:09 +0200
Subject: [PATCH 2/4] Remove unneeded comment
---
src/providers/ldap/ldap_id.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/src/providers/ldap/ldap_id.c b/src/providers/ldap/ldap_id.c
index eb43f8d096a4009c2416988b261eb3089a863696..72fdd9c260383913e1fe24bf4daa283415840d0e
100644
--- a/src/providers/ldap/ldap_id.c
+++ b/src/providers/ldap/ldap_id.c
@@ -1067,7 +1067,6 @@ void sdap_handle_account_info(struct be_req *breq, struct
sdap_id_ctx *ctx)
req = groups_by_user_send(breq, be_ctx->ev, ctx,
ar->filter_value);
if (!req) ret = ENOMEM;
- /* tevent_req_set_callback(req, groups_by_user_done, breq); */
Line was
commented: 2009-06-13 12:17:55
ACK
tevent_req_set_callback(req, sdap_account_info_initgr_done, breq);
--
1.8.2.1
From db15a991b3c8740d10120cd873f76c1efd3fa63d Mon Sep 17 00:00:00
2001
From: Jakub Hrozek <jhrozek(a)redhat.com>
Date: Fri, 17 May 2013 21:51:09 -0400
Subject: [PATCH 3/4] FO: Fix setting status of duplicates
---
src/providers/fail_over.c | 24 ++++++++++++++++--------
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/src/providers/fail_over.c b/src/providers/fail_over.c
index c30ead2b3da4b7901807d21eeed96822282935e3..cd36327d8bbcd940389fd41315be71c3aada15e3
100644
--- a/src/providers/fail_over.c
+++ b/src/providers/fail_over.c
@@ -658,6 +658,17 @@ static bool fo_server_match(struct fo_server *server,
return false;
}
+static bool fo_server_cmp(struct fo_server *s1, struct fo_server *s2)
+{
+ char *name = NULL;
+
+ if (s2->common != NULL) {
this function is called with
s1<-siter, s2<-server
so you check server->common != NULL,
but before this patch siter->common was checked.
+ name = s2->common->name;
+ }
+
+ return fo_server_match(s1, name, s2->port, s2->user_data);
+}
+
static bool fo_server_exists(struct fo_server *list,
const char *name,
int port,
@@ -1371,14 +1382,11 @@ fo_set_port_status(struct fo_server *server, enum port_status
status)
* into fo_server structures. Find the duplicates and set the same
* status */
DLIST_FOR_EACH(siter, server->service->server_list) {
- if (siter == server) continue;
- if (!siter->common || !siter->common->name) continue;
-
- if (siter->port == server->port &&
- (strcasecmp(siter->common->name, server->common->name) == 0)) {
- DEBUG(7, ("Marking port %d of duplicate server '%s' as
'%s'\n",
- siter->port, SERVER_NAME(siter),
- str_port_status(status)));
+ if (fo_server_cmp(siter, server)) {
+ DEBUG(SSSDBG_TRACE_FUNC,
+ ("Marking port %d of duplicate server '%s' as
'%s'\n",
+ siter->port, SERVER_NAME(siter),
+ str_port_status(status)));
siter->port_status = status;
gettimeofday(&siter->last_status_change, NULL);
}
--
1.8.2.1
From fae5ac11bc7c48684625b5b43d9c5856b11fa9c4 Mon Sep 17 00:00:00
2001
From: Jakub Hrozek <jhrozek(a)redhat.com>
Date: Fri, 17 May 2013 14:33:26 +0200
Subject: [PATCH 4/4] AD dyndns: extract the host name from URI
---
src/providers/ad/ad_dyndns.c | 31 ++++++++++++++++++++++---------
1 file changed, 22 insertions(+), 9 deletions(-)
diff --git a/src/providers/ad/ad_dyndns.c b/src/providers/ad/ad_dyndns.c
index 4e9ed852f91b8f99a0363b0f31af857ef98e8130..d6f2737e99b87b21f8ad6e5e1a8f008c68ee3698
100644
--- a/src/providers/ad/ad_dyndns.c
+++ b/src/providers/ad/ad_dyndns.c
@@ -159,6 +159,7 @@ static void ad_dyndns_nsupdate_done(struct tevent_req *req)
struct ad_dyndns_update_state {
struct ad_options *ad_ctx;
+ const char *servername;
};
static void ad_dyndns_sdap_update_done(struct tevent_req *subreq);
@@ -170,7 +171,7 @@ ad_dyndns_update_send(struct ad_options *ctx)
struct ad_dyndns_update_state *state;
struct tevent_req *req, *subreq;
struct sdap_id_ctx *sdap_ctx = ctx->id_ctx->sdap_id_ctx;
- const char *servername;
+ LDAPURLDesc *lud;
DEBUG(SSSDBG_TRACE_FUNC, ("Performing update\n"));
@@ -190,15 +191,27 @@ ad_dyndns_update_send(struct ad_options *ctx)
}
state->ad_ctx->dyndns_ctx->last_refresh = time(NULL);
- if (strncmp(ctx->service->sdap->uri,
- "ldap://", 7) != 0) {
- DEBUG(SSSDBG_CRIT_FAILURE, ("Unexpected format of LDAP URI.\n"));
- ret = EIO;
+ ret = ldap_url_parse(ctx->service->sdap->uri, &lud);
+ if (ret != LDAP_SUCCESS) {
+ DEBUG(SSSDBG_CRIT_FAILURE,
+ ("Failed to parse ldap URI (%s)!\n",
ctx->service->sdap->uri));
+ ret = EINVAL;
goto done;
}
- servername = ctx->service->sdap->uri + 7;
- if (!servername) {
- ret = EIO;
+
+ if (lud->lud_host == NULL) {
+ DEBUG(SSSDBG_CRIT_FAILURE,
+ ("The LDAP URI (%s) did not contain a host name\n",
+ ctx->service->sdap->uri));
+ ldap_free_urldesc(lud);
+ ret = EINVAL;
+ goto done;
+ }
+
+ state->servername = talloc_strdup(state, lud->lud_host);
+ ldap_free_urldesc(lud);
+ if (!state->servername) {
+ ret = ENOMEM;
goto done;
}
@@ -214,7 +227,7 @@ ad_dyndns_update_send(struct ad_options *ctx)
NULL,
dp_opt_get_string(ctx->basic,
AD_KRB5_REALM),
- servername,
+ state->servername,
dp_opt_get_int(ctx->dyndns_ctx->opts,
DP_OPT_DYNDNS_TTL),
false);
--
1.8.2.1
I did not test this patch, but it compiles without warnings and it looks good.
_______________________________________________
sssd-devel mailing list
sssd-devel(a)lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel