-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 12/08/2009 02:20 PM, Martin Nagy wrote:
> Unfortunately, that still doesn't solve the issue. These functions are
> surprisingly difficult to get right. The way the code is now, you don't
> link the list items. I also just noticed that we don't handle cases were
> *reply_list == NULL. Also, if we return in the middle of the function,
> we forget to free the original reply_list. Also,
> ares_free_data(reply_list); is wrong, because you're passing ares the
> double pointer, the correct way would be to pass *reply_list (and even
> better, simply old_list).
>
Thank you for the review, the issues should be fixed now.
> Seeing as these functions are so much tricky, I propose one of the
> following to be done:
>
> 1. Fix the issues outlined, create an extensive test similar to
> test_copy_hostent in resolv-test.c (for at least 2 items in the list).
> Include a special case in the test calling the function with NULL list,
> etc. Also don't forget about leak checks. Try running the test with
> mudflap to make sure we don't access any pointers that we shouldn't.
> This way, I'd be pretty confident that the code is good.
We discussed this on IRC with Martin - writing these tests is
nontrivial, because the a) the rewrite functions are static b) the ares
parsing functions allocate some private data which are freed in the
rewrite functions in our resolv code with ares_free_data.
We could work around this by shadowing ares_free_data with a dummy
function etc., but since we need these patches to build and rawhide, the
code seems to be working (for manual testing) and we currently don't use
the SRV and TXT parsing anywhere, this could be a post-1.0 task.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
iEYEARECAAYFAkseoLMACgkQHsardTLnvCUCtwCg0szmgyR3mkLTxBVrBI2Dggh7
4aMAnR8754TdW/Xqhz1fwIP43tWjQdIJ
=ybc7
-----END PGP SIGNATURE-----
From 26c68567f0516cc528b784b1b216f48a433a29ee Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhro...@redhat.com>
Date: Tue, 3 Nov 2009 16:24:25 +0100
Subject: [PATCH 1/3] Change ares usage to be c-ares 1.7.0 compatible
* Rename structure accordingly to ares upstream
* Use new ares parsing functions in the wrappers
* fix tests for ares 1.7
---
server/resolv/async_resolv.c | 148 ++++++++++++++++++++++-------------------
server/resolv/async_resolv.h | 6 +-
server/tests/resolv-tests.c | 26 ++++----
3 files changed, 93 insertions(+), 87 deletions(-)
diff --git a/server/resolv/async_resolv.c b/server/resolv/async_resolv.c
index 6ac5e41..4d8a68a 100644
--- a/server/resolv/async_resolv.c
+++ b/server/resolv/async_resolv.c
@@ -43,13 +43,13 @@
#include "util/util.h"
#ifndef HAVE_ARES_PARSE_SRV
-#define ares_parse_srv_reply(abuf, alen, srv_out, nsrvreply) \
- _ares_parse_srv_reply(abuf, alen, srv_out, nsrvreply)
+#define ares_parse_srv_reply(abuf, alen, srv_out) \
+ _ares_parse_srv_reply(abuf, alen, srv_out)
#endif /* HAVE_ARES_PARSE_SRV */
#ifndef HAVE_ARES_PARSE_TXT
-#define ares_parse_txt_reply(abuf, alen, txt_out, ntxtreply) \
- _ares_parse_txt_reply(abuf, alen, txt_out, ntxtreply)
+#define ares_parse_txt_reply(abuf, alen, txt_out) \
+ _ares_parse_txt_reply(abuf, alen, txt_out)
#endif /* HAVE_ARES_PARSE_TXT */
struct fd_watch {
@@ -442,40 +442,53 @@ ares_gethostbyname_wakeup(struct tevent_req *subreq)
}
/*
- * A simple helper function that will take an array of struct srv_reply that
+ * A simple helper function that will take an array of struct ares_srv_reply that
* was allocated by malloc() in c-ares and copies it using talloc. The old one
* is freed and the talloc one is put into 'reply_list' instead.
*/
static int
-rewrite_talloc_srv_reply(TALLOC_CTX *mem_ctx, struct srv_reply **reply_list,
- int num_replies)
+rewrite_talloc_srv_reply(TALLOC_CTX *mem_ctx, struct ares_srv_reply **reply_list)
{
- int i;
- struct srv_reply *new_list;
- struct srv_reply *old_list = *reply_list;
+ struct ares_srv_reply *ptr = NULL;
+ struct ares_srv_reply *new_list = NULL;
+ struct ares_srv_reply *old_list = *reply_list;
- new_list = talloc_array(mem_ctx, struct srv_reply, num_replies);
- if (new_list == NULL) {
- return ENOMEM;
+ /* Nothing to do, but not an error */
+ if (!old_list) {
+ return EOK;
}
- /* Copy the new_list array. */
- for (i = 0; i < num_replies; i++) {
- new_list[i].weight = old_list[i].weight;
- new_list[i].priority = old_list[i].priority;
- new_list[i].port = old_list[i].port;
- new_list[i].host = talloc_strdup(new_list, old_list[i].host);
- if (new_list[i].host == NULL) {
+ /* Copy the linked list */
+ while (old_list) {
+ /* Special case for the first node */
+ if (!new_list) {
+ new_list = talloc_zero(mem_ctx, struct ares_srv_reply);
+ if (new_list == NULL) {
+ return ENOMEM;
+ }
+ ptr = new_list;
+ } else {
+ ptr->next = talloc_zero(new_list, struct ares_srv_reply);
+ if (ptr == NULL) {
+ return ENOMEM;
+ }
+ ptr = ptr->next;
+ }
+
+ ptr->weight = old_list->weight;
+ ptr->priority = old_list->priority;
+ ptr->port = old_list->port;
+ ptr->host = talloc_strdup(ptr, old_list->host);
+ if (ptr->host == NULL) {
talloc_free(new_list);
return ENOMEM;
}
+
+ old_list = old_list->next;
}
/* Free the old one (uses malloc). */
- for (i = 0; i < num_replies; i++) {
- free(old_list[i].host);
- }
- free(old_list);
+ ares_free_data(*reply_list);
/* And now put our own new_list in place. */
*reply_list = new_list;
@@ -493,8 +506,7 @@ struct getsrv_state {
const char *query;
/* parsed data returned by ares */
- struct srv_reply *reply_list;
- int num_replies;
+ struct ares_srv_reply *reply_list;
int status;
int timeouts;
};
@@ -522,7 +534,6 @@ resolv_getsrv_send(TALLOC_CTX *mem_ctx, struct tevent_context *ev,
state->resolv_ctx = ctx;
state->query = query;
state->reply_list = NULL;
- state->num_replies = 0;
state->status = 0;
state->timeouts = 0;
@@ -543,8 +554,7 @@ resolv_getsrv_done(void *arg, int status, int timeouts, unsigned char *abuf, int
struct tevent_req *req = talloc_get_type(arg, struct tevent_req);
struct getsrv_state *state = tevent_req_data(req, struct getsrv_state);
int ret;
- int num_replies;
- struct srv_reply *reply_list;
+ struct ares_srv_reply *reply_list;
state->status = status;
state->timeouts = timeouts;
@@ -555,32 +565,29 @@ resolv_getsrv_done(void *arg, int status, int timeouts, unsigned char *abuf, int
goto fail;
}
- ret = ares_parse_srv_reply(abuf, alen, &reply_list, &num_replies);
+ ret = ares_parse_srv_reply(abuf, alen, &reply_list);
if (status != ARES_SUCCESS) {
DEBUG(2, ("SRV record parsing failed: %d: %s\n", ret, ares_strerror(ret)));
ret = return_code(ret);
goto fail;
}
- ret = rewrite_talloc_srv_reply(req, &reply_list, num_replies);
+ ret = rewrite_talloc_srv_reply(req, &reply_list);
if (ret != EOK) {
goto fail;
}
state->reply_list = reply_list;
- state->num_replies = num_replies;
tevent_req_done(req);
return;
fail:
state->reply_list = NULL;
- state->num_replies = 0;
tevent_req_error(req, ret);
}
int
resolv_getsrv_recv(TALLOC_CTX *mem_ctx, struct tevent_req *req, int *status,
- int *timeouts, struct srv_reply **reply_list,
- int *num_replies)
+ int *timeouts, struct ares_srv_reply **reply_list)
{
struct getsrv_state *state = tevent_req_data(req, struct getsrv_state);
@@ -590,8 +597,6 @@ resolv_getsrv_recv(TALLOC_CTX *mem_ctx, struct tevent_req *req, int *status,
*timeouts = state->timeouts;
if (reply_list)
*reply_list = talloc_steal(mem_ctx, state->reply_list);
- if (num_replies)
- *num_replies = state->num_replies;
TEVENT_REQ_RETURN_ON_ERROR(req);
@@ -627,34 +632,47 @@ ares_getsrv_wakeup(struct tevent_req *subreq)
* is freed and the talloc one is put into 'reply_list' instead.
*/
static int
-rewrite_talloc_txt_reply(TALLOC_CTX *mem_ctx, struct txt_reply **reply_list,
- int num_replies)
+rewrite_talloc_txt_reply(TALLOC_CTX *mem_ctx, struct ares_txt_reply **reply_list)
{
- int i;
- struct txt_reply *new_list;
- struct txt_reply *old_list = *reply_list;
+ struct ares_txt_reply *ptr = NULL;
+ struct ares_txt_reply *new_list = NULL;
+ struct ares_txt_reply *old_list = *reply_list;
- new_list = talloc_array(mem_ctx, struct txt_reply, num_replies);
- if (new_list == NULL) {
- return ENOMEM;
+ /* Nothing to do, but not an error */
+ if (!old_list) {
+ return EOK;
}
- /* Copy the new_list array. */
- for (i = 0; i < num_replies; i++) {
- new_list[i].length = old_list[i].length;
- new_list[i].txt = talloc_memdup(new_list, old_list[i].txt,
- old_list[i].length);
- if (new_list[i].txt == NULL) {
+ /* Copy the linked list */
+ while (old_list) {
+
+ /* Special case for the first node */
+ if (!new_list) {
+ new_list = talloc_zero(mem_ctx, struct ares_txt_reply);
+ if (new_list == NULL) {
+ return ENOMEM;
+ }
+ ptr = new_list;
+ } else {
+ ptr->next = talloc_zero(new_list, struct ares_txt_reply);
+ if (ptr == NULL) {
+ return ENOMEM;
+ }
+ ptr = ptr->next;
+ }
+
+ ptr->length = old_list->length;
+ ptr->txt = talloc_memdup(ptr, old_list->txt,
+ old_list->length);
+ if (ptr->txt == NULL) {
talloc_free(new_list);
return ENOMEM;
}
- }
- /* Free the old one (uses malloc). */
- for (i = 0; i < num_replies; i++) {
- free(old_list[i].txt);
+ old_list = old_list->next;
}
- free(old_list);
+
+ ares_free_data(*reply_list);
/* And now put our own new_list in place. */
*reply_list = new_list;
@@ -672,8 +690,7 @@ struct gettxt_state {
const char *query;
/* parsed data returned by ares */
- struct txt_reply *reply_list;
- int num_replies;
+ struct ares_txt_reply *reply_list;
int status;
int timeouts;
};
@@ -701,7 +718,6 @@ resolv_gettxt_send(TALLOC_CTX *mem_ctx, struct tevent_context *ev,
state->resolv_ctx = ctx;
state->query = query;
state->reply_list = NULL;
- state->num_replies = 0;
state->status = 0;
state->timeouts = 0;
@@ -723,8 +739,7 @@ resolv_gettxt_done(void *arg, int status, int timeouts, unsigned char *abuf, int
struct tevent_req *req = talloc_get_type(arg, struct tevent_req);
struct gettxt_state *state = tevent_req_data(req, struct gettxt_state);
int ret;
- int num_replies;
- struct txt_reply *reply_list;
+ struct ares_txt_reply *reply_list;
state->status = status;
state->timeouts = timeouts;
@@ -734,32 +749,29 @@ resolv_gettxt_done(void *arg, int status, int timeouts, unsigned char *abuf, int
goto fail;
}
- ret = ares_parse_txt_reply(abuf, alen, &reply_list, &num_replies);
+ ret = ares_parse_txt_reply(abuf, alen, &reply_list);
if (status != ARES_SUCCESS) {
DEBUG(2, ("TXT record parsing failed: %d: %s\n", ret, ares_strerror(ret)));
ret = return_code(ret);
goto fail;
}
- ret = rewrite_talloc_txt_reply(req, &reply_list, num_replies);
+ ret = rewrite_talloc_txt_reply(req, &reply_list);
if (ret != EOK) {
goto fail;
}
state->reply_list = reply_list;
- state->num_replies = num_replies;
tevent_req_done(req);
return;
fail:
state->reply_list = NULL;
- state->num_replies = 0;
tevent_req_error(req, ret);
}
int
resolv_gettxt_recv(TALLOC_CTX *mem_ctx, struct tevent_req *req, int *status,
- int *timeouts, struct txt_reply **reply_list,
- int *num_replies)
+ int *timeouts, struct ares_txt_reply **reply_list)
{
struct gettxt_state *state = tevent_req_data(req, struct gettxt_state);
@@ -769,8 +781,6 @@ resolv_gettxt_recv(TALLOC_CTX *mem_ctx, struct tevent_req *req, int *status,
*timeouts = state->timeouts;
if (reply_list)
*reply_list = talloc_steal(mem_ctx, state->reply_list);
- if (num_replies)
- *num_replies = state->num_replies;
TEVENT_REQ_RETURN_ON_ERROR(req);
diff --git a/server/resolv/async_resolv.h b/server/resolv/async_resolv.h
index 702d4dd..9d080ec 100644
--- a/server/resolv/async_resolv.h
+++ b/server/resolv/async_resolv.h
@@ -77,8 +77,7 @@ int resolv_getsrv_recv(TALLOC_CTX *mem_ctx,
struct tevent_req *req,
int *status,
int *timeouts,
- struct srv_reply **reply_list,
- int *num_replies);
+ struct ares_srv_reply **reply_list);
/** Get TXT record **/
struct tevent_req *resolv_gettxt_send(TALLOC_CTX *mem_ctx,
@@ -90,7 +89,6 @@ int resolv_gettxt_recv(TALLOC_CTX *mem_ctx,
struct tevent_req *req,
int *status,
int *timeouts,
- struct txt_reply **reply_list,
- int *num_replies);
+ struct ares_txt_reply **reply_list);
#endif /* __ASYNC_RESOLV_H__ */
diff --git a/server/tests/resolv-tests.c b/server/tests/resolv-tests.c
index bf76849..d6b8c4f 100644
--- a/server/tests/resolv-tests.c
+++ b/server/tests/resolv-tests.c
@@ -253,15 +253,13 @@ END_TEST
static void test_internet(struct tevent_req *req)
{
- int i;
int recv_status;
int status;
struct resolv_test_ctx *test_ctx;
void *tmp_ctx;
struct hostent *hostent = NULL;
- struct txt_reply *txt_replies = NULL;
- struct srv_reply *srv_replies = NULL;
- int count;
+ struct ares_txt_reply *txt_replies = NULL, *txtptr;
+ struct ares_srv_reply *srv_replies = NULL, *srvptr;
test_ctx = tevent_req_callback_data(req, struct resolv_test_ctx);
@@ -278,20 +276,20 @@ static void test_internet(struct tevent_req *req)
break;
case TESTING_TXT:
recv_status = resolv_gettxt_recv(tmp_ctx, req, &status, NULL,
- &txt_replies, &count);
- test_ctx->error = (count == 0) ? ENOENT : EOK;
- for (i = 0; i < count; i++) {
- DEBUG(2, ("TXT Record: %s\n", txt_replies[i].txt));
+ &txt_replies);
+ test_ctx->error = (txt_replies == NULL) ? ENOENT : EOK;
+ for (txtptr = txt_replies; txtptr != NULL; txtptr = txtptr->next) {
+ DEBUG(2, ("TXT Record: %s\n", txtptr->txt));
}
break;
case TESTING_SRV:
recv_status = resolv_getsrv_recv(tmp_ctx, req, &status, NULL,
- &srv_replies, &count);
- test_ctx->error = (count == 0) ? ENOENT : EOK;
- for (i = 0; i < count; i++) {
- DEBUG(2, ("SRV Record: %d %d %d %s\n", srv_replies[i].weight,
- srv_replies[i].priority, srv_replies[i].port,
- srv_replies[i].host));
+ &srv_replies);
+ test_ctx->error = (srv_replies == NULL) ? ENOENT : EOK;
+ for (srvptr = srv_replies; srvptr != NULL; srvptr = srvptr->next) {
+ DEBUG(2, ("SRV Record: %d %d %d %s\n", srvptr->weight,
+ srvptr->priority, srvptr->port,
+ srvptr->host));
}
break;
}
--
1.6.2.5
From f3f9796b5b94565abcc580429e946099fdcd0505 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhro...@redhat.com>
Date: Fri, 27 Nov 2009 09:39:41 +0100
Subject: [PATCH 2/3] Import ares 1.7.0 helpers
---
server/Makefile.am | 11 ++-
server/external/libcares.m4 | 21 +----
server/resolv/ares/ares_data.c | 140 +++++++++++++++++++++++++++++
server/resolv/ares/ares_data.h | 68 ++++++++++++++
server/resolv/ares/ares_parse_srv_reply.c | 88 ++++++++++--------
server/resolv/ares/ares_parse_srv_reply.h | 13 ++--
server/resolv/ares/ares_parse_txt_reply.c | 119 +++++++++++++++++--------
server/resolv/ares/ares_parse_txt_reply.h | 9 +-
server/resolv/async_resolv.c | 11 ++-
server/resolv/async_resolv.h | 10 +-
10 files changed, 374 insertions(+), 116 deletions(-)
create mode 100644 server/resolv/ares/ares_data.c
create mode 100644 server/resolv/ares/ares_data.h
diff --git a/server/Makefile.am b/server/Makefile.am
index c43eb47..3902da1 100644
--- a/server/Makefile.am
+++ b/server/Makefile.am
@@ -209,11 +209,11 @@ SSSD_TOOLS_OBJ = \
SSSD_RESOLV_OBJ = \
resolv/async_resolv.c
-if BUILD_ARES_PARSE_SRV
- SSSD_RESOLV_OBJ += resolv/ares/ares_parse_srv_reply.c
-endif
-if BUILD_ARES_PARSE_TXT
- SSSD_RESOLV_OBJ += resolv/ares/ares_parse_txt_reply.c
+if BUILD_ARES_DATA
+ SSSD_RESOLV_OBJ += \
+ resolv/ares/ares_parse_srv_reply.c \
+ resolv/ares/ares_parse_txt_reply.c \
+ resolv/ares/ares_data.c
endif
SSSD_FAILOVER_OBJ = \
@@ -309,6 +309,7 @@ dist_noinst_HEADERS = \
resolv/async_resolv.h \
resolv/ares/ares_parse_srv_reply.h \
resolv/ares/ares_parse_txt_reply.h \
+ resolv/ares/ares_data.h \
tests/common.h
diff --git a/server/external/libcares.m4 b/server/external/libcares.m4
index 020a170..657deac 100644
--- a/server/external/libcares.m4
+++ b/server/external/libcares.m4
@@ -7,25 +7,14 @@ AC_CHECK_HEADERS(ares.h,
[AC_MSG_ERROR([c-ares header files are not installed])]
)
-dnl Check if this particular version of c-ares supports parsing of SRV records
+dnl Check if this particular version of c-ares supports the generic ares_free_data function
AC_CHECK_LIB([cares],
- [ares_parse_srv_reply],
- [AC_DEFINE([HAVE_ARES_PARSE_SRV], 1, [Does c-ares support srv parsing?])
+ [ares_free_data],
+ [AC_DEFINE([HAVE_ARES_DATA], 1, [Does c-ares have ares_free_data()?])
],
[
- ares_build_srv=1
+ ares_data=1
]
)
-dnl Check if this particular version of c-ares supports parsing of TXT records
-AC_CHECK_LIB([cares],
- [ares_parse_txt_reply],
- [AC_DEFINE([HAVE_ARES_PARSE_TXT], 1, [Does c-ares support txt parsing?])
- ],
- [
- ares_build_txt=1
- ]
-)
-
-AM_CONDITIONAL(BUILD_ARES_PARSE_SRV, test x$ares_build_srv = x1)
-AM_CONDITIONAL(BUILD_ARES_PARSE_TXT, test x$ares_build_txt = x1)
+AM_CONDITIONAL(BUILD_ARES_DATA, test x$ares_data = x1)
diff --git a/server/resolv/ares/ares_data.c b/server/resolv/ares/ares_data.c
new file mode 100644
index 0000000..1cccaa5
--- /dev/null
+++ b/server/resolv/ares/ares_data.c
@@ -0,0 +1,140 @@
+/* $Id: ares_data.c,v 1.2 2009-11-20 09:06:33 yangtse Exp $ */
+
+/* Copyright (C) 2009 by Daniel Stenberg
+ *
+ * Permission to use, copy, modify, and distribute this
+ * software and its documentation for any purpose and without
+ * fee is hereby granted, provided that the above copyright
+ * notice appear in all copies and that both that copyright
+ * notice and this permission notice appear in supporting
+ * documentation, and that the name of M.I.T. not be used in
+ * advertising or publicity pertaining to distribution of the
+ * software without specific, written prior permission.
+ * M.I.T. makes no representations about the suitability of
+ * this software for any purpose. It is provided "as is"
+ * without express or implied warranty.
+ */
+
+
+#include <stddef.h>
+#include <stdlib.h>
+
+#include "ares.h"
+#include "ares_data.h"
+
+/*
+** ares_free_data() - c-ares external API function.
+**
+** This function must be used by the application to free data memory that
+** has been internally allocated by some c-ares function and for which a
+** pointer has already been returned to the calling application. The list
+** of c-ares functions returning pointers that must be free'ed using this
+** function is:
+**
+** ares_parse_srv_reply()
+** ares_parse_txt_reply()
+*/
+
+void _ares_free_data(void *dataptr)
+{
+ struct ares_data *ptr;
+
+ if (!dataptr)
+ return;
+
+ ptr = (void *)((char *)dataptr - offsetof(struct ares_data, data));
+
+ if (ptr->mark != ARES_DATATYPE_MARK)
+ return;
+
+ switch (ptr->type)
+ {
+ case ARES_DATATYPE_SRV_REPLY:
+
+ if (ptr->data.srv_reply.next)
+ _ares_free_data(ptr->data.srv_reply.next);
+ if (ptr->data.srv_reply.host)
+ free(ptr->data.srv_reply.host);
+ break;
+
+ case ARES_DATATYPE_TXT_REPLY:
+
+ if (ptr->data.txt_reply.next)
+ _ares_free_data(ptr->data.txt_reply.next);
+ if (ptr->data.txt_reply.txt)
+ free(ptr->data.txt_reply.txt);
+ break;
+
+ default:
+ return;
+ }
+
+ free(ptr);
+}
+
+
+/*
+** ares_malloc_data() - c-ares internal helper function.
+**
+** This function allocates memory for a c-ares private ares_data struct
+** for the specified ares_datatype, initializes c-ares private fields
+** and zero initializes those which later might be used from the public
+** API. It returns an interior pointer which can be passed by c-ares
+** functions to the calling application, and that must be free'ed using
+** c-ares external API function ares_free_data().
+*/
+
+void *_ares_malloc_data(ares_datatype type)
+{
+ struct ares_data *ptr;
+
+ ptr = malloc(sizeof(struct ares_data));
+ if (!ptr)
+ return NULL;
+
+ switch (type)
+ {
+ case ARES_DATATYPE_SRV_REPLY:
+ ptr->data.srv_reply.next = NULL;
+ ptr->data.srv_reply.host = NULL;
+ ptr->data.srv_reply.priority = 0;
+ ptr->data.srv_reply.weight = 0;
+ ptr->data.srv_reply.port = 0;
+ break;
+
+ case ARES_DATATYPE_TXT_REPLY:
+ ptr->data.txt_reply.next = NULL;
+ ptr->data.txt_reply.txt = NULL;
+ ptr->data.txt_reply.length = 0;
+ break;
+
+ default:
+ free(ptr);
+ return NULL;
+ }
+
+ ptr->mark = ARES_DATATYPE_MARK;
+ ptr->type = type;
+
+ return &ptr->data;
+}
+
+
+/*
+** ares_get_datatype() - c-ares internal helper function.
+**
+** This function returns the ares_datatype of the data stored in a
+** private ares_data struct when given the public API pointer.
+*/
+
+ares_datatype ares_get_datatype(void * dataptr)
+{
+ struct ares_data *ptr;
+
+ ptr = (void *)((char *)dataptr - offsetof(struct ares_data, data));
+
+ if (ptr->mark == ARES_DATATYPE_MARK)
+ return ptr->type;
+
+ return ARES_DATATYPE_UNKNOWN;
+}
diff --git a/server/resolv/ares/ares_data.h b/server/resolv/ares/ares_data.h
new file mode 100644
index 0000000..d360631
--- /dev/null
+++ b/server/resolv/ares/ares_data.h
@@ -0,0 +1,68 @@
+/* $Id: ares_data.h,v 1.2 2009-11-23 12:03:33 yangtse Exp $ */
+
+/* Copyright (C) 2009 by Daniel Stenberg
+ *
+ * Permission to use, copy, modify, and distribute this
+ * software and its documentation for any purpose and without
+ * fee is hereby granted, provided that the above copyright
+ * notice appear in all copies and that both that copyright
+ * notice and this permission notice appear in supporting
+ * documentation, and that the name of M.I.T. not be used in
+ * advertising or publicity pertaining to distribution of the
+ * software without specific, written prior permission.
+ * M.I.T. makes no representations about the suitability of
+ * this software for any purpose. It is provided "as is"
+ * without express or implied warranty.
+ */
+
+#ifndef HAVE_ARES_DATA
+#include "resolv/ares/ares_parse_txt_reply.h"
+#include "resolv/ares/ares_parse_srv_reply.h"
+#endif /* HAVE_ARES_DATA */
+
+typedef enum {
+ ARES_DATATYPE_UNKNOWN = 1, /* unknown data type - introduced in 1.7.0 */
+ ARES_DATATYPE_SRV_REPLY, /* struct ares_srv_reply - introduced in 1.7.0 */
+ ARES_DATATYPE_TXT_REPLY, /* struct ares_txt_reply - introduced in 1.7.0 */
+#if 0
+ ARES_DATATYPE_ADDR6TTL, /* struct ares_addrttl */
+ ARES_DATATYPE_ADDRTTL, /* struct ares_addr6ttl */
+ ARES_DATATYPE_HOSTENT, /* struct hostent */
+ ARES_DATATYPE_OPTIONS, /* struct ares_options */
+#endif
+ ARES_DATATYPE_LAST /* not used - introduced in 1.7.0 */
+} ares_datatype;
+
+#define ARES_DATATYPE_MARK 0xbead
+
+/*
+ * ares_data struct definition is internal to c-ares and shall not
+ * be exposed by the public API in order to allow future changes
+ * and extensions to it without breaking ABI. This will be used
+ * internally by c-ares as the container of multiple types of data
+ * dynamically allocated for which a reference will be returned
+ * to the calling application.
+ *
+ * c-ares API functions returning a pointer to c-ares internally
+ * allocated data will actually be returning an interior pointer
+ * into this ares_data struct.
+ *
+ * All this is 'invisible' to the calling application, the only
+ * requirement is that this kind of data must be free'ed by the
+ * calling application using ares_free_data() with the pointer
+ * it has received from a previous c-ares function call.
+ */
+
+struct ares_data {
+ ares_datatype type; /* Actual data type identifier. */
+ unsigned int mark; /* Private ares_data signature. */
+ union {
+ struct ares_txt_reply txt_reply;
+ struct ares_srv_reply srv_reply;
+ } data;
+};
+
+void *_ares_malloc_data(ares_datatype type);
+void _ares_free_data(void *dataptr);
+
+ares_datatype ares_get_datatype(void * dataptr);
diff --git a/server/resolv/ares/ares_parse_srv_reply.c b/server/resolv/ares/ares_parse_srv_reply.c
index 9745fb0..086c4db 100644
--- a/server/resolv/ares/ares_parse_srv_reply.c
+++ b/server/resolv/ares/ares_parse_srv_reply.c
@@ -51,25 +51,24 @@
#include "ares.h"
/* this drags in some private macros c-ares uses */
#include "ares_dns.h"
+#include "ares_data.h"
#include "ares_parse_srv_reply.h"
int _ares_parse_srv_reply (const unsigned char *abuf, int alen,
- struct srv_reply **srv_out, int *nsrvreply)
+ struct ares_srv_reply **srv_out)
{
- unsigned int qdcount, ancount;
- const unsigned char *aptr;
- int status, i, rr_type, rr_class, rr_len;
+ unsigned int qdcount, ancount, i;
+ const unsigned char *aptr, *vptr;
+ int status, rr_type, rr_class, rr_len;
long len;
char *hostname = NULL, *rr_name = NULL;
- struct srv_reply *srv = NULL;
+ struct ares_srv_reply *srv_head = NULL;
+ struct ares_srv_reply *srv_last = NULL;
+ struct ares_srv_reply *srv_curr;
/* Set *srv_out to NULL for all failure cases. */
- if (srv_out)
- *srv_out = NULL;
- /* Same with *nsrvreply. */
- if (nsrvreply)
- *nsrvreply = 0;
+ *srv_out = NULL;
/* Give up if abuf doesn't have room for a header. */
if (alen < HFIXEDSZ)
@@ -96,14 +95,6 @@ int _ares_parse_srv_reply (const unsigned char *abuf, int alen,
}
aptr += len + QFIXEDSZ;
- /* Allocate srv_reply array; ancount gives an upper bound */
- srv = malloc ((ancount) * sizeof (struct srv_reply));
- if (!srv)
- {
- free (hostname);
- return ARES_ENOMEM;
- }
-
/* Examine each answer resource record (RR) in turn. */
for (i = 0; i < (int) ancount; i++)
{
@@ -134,40 +125,59 @@ int _ares_parse_srv_reply (const unsigned char *abuf, int alen,
break;
}
- srv[i].priority = ntohs (*((const uint16_t *)aptr));
- aptr += sizeof(uint16_t);
- srv[i].weight = ntohs (*((const uint16_t *)aptr));
- aptr += sizeof(uint16_t);
- srv[i].port = ntohs (*((const uint16_t *)aptr));
- aptr += sizeof(uint16_t);
+ /* Allocate storage for this SRV answer appending it to the list */
+ srv_curr = _ares_malloc_data(ARES_DATATYPE_SRV_REPLY);
+ if (!srv_curr)
+ {
+ status = ARES_ENOMEM;
+ break;
+ }
+ if (srv_last)
+ {
+ srv_last->next = srv_curr;
+ }
+ else
+ {
+ srv_head = srv_curr;
+ }
+ srv_last = srv_curr;
+
+ vptr = aptr;
+ srv_curr->priority = ntohs (*((const unsigned short *)vptr));
+ vptr += sizeof(const unsigned short);
+ srv_curr->weight = ntohs (*((const unsigned short *)vptr));
+ vptr += sizeof(const unsigned short);
+ srv_curr->port = ntohs (*((const unsigned short *)vptr));
+ vptr += sizeof(const unsigned short);
- status = ares_expand_name (aptr, abuf, alen, &srv[i].host, &len);
+ status = ares_expand_name (vptr, abuf, alen, &srv_curr->host, &len);
if (status != ARES_SUCCESS)
break;
+ }
- /* Move on to the next record */
- aptr += len;
+ /* Don't lose memory in the next iteration */
+ free(rr_name);
+ rr_name = NULL;
- /* Don't lose memory in the next iteration */
- free (rr_name);
- rr_name = NULL;
- }
+ /* Move on to the next record */
+ aptr += rr_len;
}
+ if (hostname)
+ free (hostname);
+ if (rr_name)
+ free (rr_name);
+
/* clean up on error */
if (status != ARES_SUCCESS)
{
- free (srv);
- free (hostname);
- free (rr_name);
+ if (srv_head)
+ _ares_free_data (srv_head);
return status;
}
/* everything looks fine, return the data */
- *srv_out = srv;
- *nsrvreply = ancount;
+ *srv_out = srv_head;
- free (hostname);
- free (rr_name);
- return status;
+ return ARES_SUCCESS;
}
diff --git a/server/resolv/ares/ares_parse_srv_reply.h b/server/resolv/ares/ares_parse_srv_reply.h
index caf93cd..29c6e08 100644
--- a/server/resolv/ares/ares_parse_srv_reply.h
+++ b/server/resolv/ares/ares_parse_srv_reply.h
@@ -21,14 +21,15 @@
#ifndef __ARES_PARSE_SRV_REPLY_H__
#define __ARES_PARSE_SRV_REPLY_H__
-struct srv_reply {
- u_int16_t weight;
- u_int16_t priority;
- u_int16_t port;
- char *host;
+struct ares_srv_reply {
+ struct ares_srv_reply *next;
+ char *host;
+ unsigned short priority;
+ unsigned short weight;
+ unsigned short port;
};
int _ares_parse_srv_reply (const unsigned char *abuf, int alen,
- struct srv_reply **srv_out, int *nsrvreply);
+ struct ares_srv_reply **srv_out);
#endif /* __ARES_PARSE_SRV_REPLY_H__ */
diff --git a/server/resolv/ares/ares_parse_txt_reply.c b/server/resolv/ares/ares_parse_txt_reply.c
index feb6af2..d710e8f 100644
--- a/server/resolv/ares/ares_parse_txt_reply.c
+++ b/server/resolv/ares/ares_parse_txt_reply.c
@@ -50,21 +50,26 @@
#include "ares.h"
/* this drags in some private macros c-ares uses */
#include "ares_dns.h"
+#include "ares_data.h"
#include "ares_parse_txt_reply.h"
-int _ares_parse_txt_reply(const unsigned char* abuf, int alen,
- struct txt_reply **txt_out, int *ntxtreply)
+int _ares_parse_txt_reply (const unsigned char *abuf, int alen,
+ struct ares_txt_reply **txt_out)
{
- unsigned int qdcount, ancount;
+ size_t substr_len, str_len;
+ unsigned int qdcount, ancount, i;
const unsigned char *aptr;
- int status, i, rr_type, rr_class, rr_len;
+ const unsigned char *strptr;
+ int status, rr_type, rr_class, rr_len;
long len;
char *hostname = NULL, *rr_name = NULL;
- struct txt_reply *txt = NULL;
+ struct ares_txt_reply *txt_head = NULL;
+ struct ares_txt_reply *txt_last = NULL;
+ struct ares_txt_reply *txt_curr;
- if (txt_out)
- *txt_out = NULL;
+ /* Set *txt_out to NULL for all failure cases. */
+ *txt_out = NULL;
/* Give up if abuf doesn't have room for a header. */
if (alen < HFIXEDSZ)
@@ -91,29 +96,21 @@ int _ares_parse_txt_reply(const unsigned char* abuf, int alen,
}
aptr += len + QFIXEDSZ;
- /* Allocate txt_reply array; ancount gives an upper bound */
- txt = malloc ((ancount) * sizeof (struct txt_reply));
- if (!txt)
- {
- free (hostname);
- return ARES_ENOMEM;
- }
-
/* Examine each answer resource record (RR) in turn. */
for (i = 0; i < (int) ancount; i++)
{
/* Decode the RR up to the data field. */
status = ares_expand_name(aptr, abuf, alen, &rr_name, &len);
if (status != ARES_SUCCESS)
- {
- break;
- }
+ {
+ break;
+ }
aptr += len;
if (aptr + RRFIXEDSZ > abuf + alen)
- {
- status = ARES_EBADRESP;
- break;
- }
+ {
+ status = ARES_EBADRESP;
+ break;
+ }
rr_type = DNS_RR_TYPE(aptr);
rr_class = DNS_RR_CLASS(aptr);
rr_len = DNS_RR_LEN(aptr);
@@ -121,37 +118,87 @@ int _ares_parse_txt_reply(const unsigned char* abuf, int alen,
/* Check if we are really looking at a TXT record */
if (rr_class == C_IN && rr_type == T_TXT)
- {
- /* Grab the TXT payload */
- txt[i].length = rr_len;
- txt[i].txt = malloc(sizeof(unsigned char) * rr_len);
- if (txt[i].txt == NULL)
+ {
+ /* Allocate storage for this TXT answer appending it to the list */
+ txt_curr = _ares_malloc_data(ARES_DATATYPE_TXT_REPLY);
+ if (!txt_curr)
{
status = ARES_ENOMEM;
break;
}
- memcpy((void *) txt[i].txt, aptr+1, sizeof(unsigned char) * rr_len);
- /* Move on to the next record */
- aptr += rr_len;
+ if (txt_last)
+ {
+ txt_last->next = txt_curr;
+ }
+ else
+ {
+ txt_head = txt_curr;
+ }
+ txt_last = txt_curr;
+
+ /*
+ * There may be multiple substrings in a single TXT record. Each
+ * substring may be up to 255 characters in length, with a
+ * "length byte" indicating the size of the substring payload.
+ * RDATA contains both the length-bytes and payloads of all
+ * substrings contained therein.
+ */
+
+ /* Compute total length to allow a single memory allocation */
+ strptr = aptr;
+ while (strptr < (aptr + rr_len))
+ {
+ substr_len = (unsigned char)*strptr;
+ txt_curr->length += substr_len;
+ strptr += substr_len + 1;
+ }
+
+ /* Including null byte */
+ txt_curr->txt = malloc (txt_curr->length + 1);
+ if (txt_curr->txt == NULL)
+ {
+ status = ARES_ENOMEM;
+ break;
+ }
+
+ /* Step through the list of substrings, concatenating them */
+ str_len = 0;
+ strptr = aptr;
+ while (strptr < (aptr + rr_len))
+ {
+ substr_len = (unsigned char)*strptr;
+ strptr++;
+ memcpy ((char *) txt_curr->txt + str_len, strptr, substr_len);
+ str_len += substr_len;
+ strptr += substr_len;
+ }
+ /* Make sure we NULL-terminate */
+ *((char *) txt_curr->txt + txt_curr->length) = '\0';
}
/* Don't lose memory in the next iteration */
free(rr_name);
rr_name = NULL;
+
+ /* Move on to the next record */
+ aptr += rr_len;
}
- free(hostname);
- free(rr_name);
+ if (hostname)
+ free (hostname);
+ if (rr_name)
+ free (rr_name);
/* clean up on error */
if (status != ARES_SUCCESS)
{
- free (txt);
+ if (txt_head)
+ _ares_free_data (txt_head);
return status;
}
/* everything looks fine, return the data */
- *txt_out = txt;
- *ntxtreply = ancount;
- return 0;
+ *txt_out = txt_head;
+
+ return ARES_SUCCESS;
}
diff --git a/server/resolv/ares/ares_parse_txt_reply.h b/server/resolv/ares/ares_parse_txt_reply.h
index 9fd4278..216e2c0 100644
--- a/server/resolv/ares/ares_parse_txt_reply.h
+++ b/server/resolv/ares/ares_parse_txt_reply.h
@@ -21,12 +21,13 @@
#ifndef __ARES_PARSE_TXT_REPLY_H__
#define __ARES_PARSE_TXT_REPLY_H__
-struct txt_reply {
- int length; /* length of the text */
- unsigned char *txt; /* may contain nulls */
+struct ares_txt_reply {
+ struct ares_txt_reply *next;
+ unsigned char *txt;
+ size_t length; /* length excludes null termination */
};
int _ares_parse_txt_reply(const unsigned char* abuf, int alen,
- struct txt_reply **txt_out, int *ntxtreply);
+ struct ares_txt_reply **txt_out);
#endif /* __ARES_PARSE_TXT_REPLY_H__ */
diff --git a/server/resolv/async_resolv.c b/server/resolv/async_resolv.c
index 4d8a68a..5058d74 100644
--- a/server/resolv/async_resolv.c
+++ b/server/resolv/async_resolv.c
@@ -42,15 +42,16 @@
#include "util/dlinklist.h"
#include "util/util.h"
-#ifndef HAVE_ARES_PARSE_SRV
+#ifndef HAVE_ARES_DATA
#define ares_parse_srv_reply(abuf, alen, srv_out) \
_ares_parse_srv_reply(abuf, alen, srv_out)
-#endif /* HAVE_ARES_PARSE_SRV */
-
-#ifndef HAVE_ARES_PARSE_TXT
#define ares_parse_txt_reply(abuf, alen, txt_out) \
_ares_parse_txt_reply(abuf, alen, txt_out)
-#endif /* HAVE_ARES_PARSE_TXT */
+#define ares_free_data(dataptr) \
+ _ares_free_data(dataptr)
+#define ares_malloc_data(data) \
+ _ares_malloc_data(data)
+#endif /* HAVE_ARES_DATA */
struct fd_watch {
struct fd_watch *prev;
diff --git a/server/resolv/async_resolv.h b/server/resolv/async_resolv.h
index 9d080ec..dab2fdf 100644
--- a/server/resolv/async_resolv.h
+++ b/server/resolv/async_resolv.h
@@ -29,13 +29,13 @@
#include <netdb.h>
#include <ares.h>
-#ifndef HAVE_ARES_PARSE_TXT
-#include "resolv/ares/ares_parse_txt_reply.h"
-#endif /* HAVE_ARES_PARSE_TXT */
+#include "config.h"
-#ifndef HAVE_ARES_PARSE_SRV
+#ifndef HAVE_ARES_DATA
#include "resolv/ares/ares_parse_srv_reply.h"
-#endif /* HAVE_ARES_PARSE_SRV */
+#include "resolv/ares/ares_parse_txt_reply.h"
+#include "resolv/ares/ares_data.h"
+#endif /* HAVE_ARES_DATA */
/*
* An opaque structure which holds context for a module using the async
--
1.6.2.5
From 08f618f9266a602fc9c562a3e49ba7abf008100d Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhro...@redhat.com>
Date: Mon, 7 Dec 2009 19:28:33 +0100
Subject: [PATCH 3/3] Don't build the SRV and TXT parsing code except for tests
---
server/Makefile.am | 20 ++++++++++++--------
server/resolv/async_resolv.c | 6 ++++++
2 files changed, 18 insertions(+), 8 deletions(-)
diff --git a/server/Makefile.am b/server/Makefile.am
index 3902da1..ed6f0de 100644
--- a/server/Makefile.am
+++ b/server/Makefile.am
@@ -209,12 +209,6 @@ SSSD_TOOLS_OBJ = \
SSSD_RESOLV_OBJ = \
resolv/async_resolv.c
-if BUILD_ARES_DATA
- SSSD_RESOLV_OBJ += \
- resolv/ares/ares_parse_srv_reply.c \
- resolv/ares/ares_parse_txt_reply.c \
- resolv/ares/ares_data.c
-endif
SSSD_FAILOVER_OBJ = \
providers/fail_over.c \
@@ -478,14 +472,24 @@ files_tests_CFLAGS = \
files_tests_LDADD = \
$(FILES_TESTS_LIBS)
+SSSD_RESOLV_TESTS_OBJ = \
+ $(SSSD_RESOLV_OBJ)
+if BUILD_ARES_DATA
+ SSSD_RESOLV_TESTS_OBJ += \
+ resolv/ares/ares_parse_srv_reply.c \
+ resolv/ares/ares_parse_txt_reply.c \
+ resolv/ares/ares_data.c
+endif
+
resolv_tests_SOURCES = \
tests/resolv-tests.c \
tests/common.c \
$(SSSD_UTIL_OBJ) \
- $(SSSD_RESOLV_OBJ)
+ $(SSSD_RESOLV_TESTS_OBJ)
resolv_tests_CFLAGS = \
$(AM_CFLAGS) \
- $(CHECK_CFLAGS)
+ $(CHECK_CFLAGS) \
+ -DBUILD_TXT_SRV
resolv_tests_LDADD = \
$(SSSD_LIBS) \
$(CHECK_LIBS) \
diff --git a/server/resolv/async_resolv.c b/server/resolv/async_resolv.c
index 5058d74..7e01eaf 100644
--- a/server/resolv/async_resolv.c
+++ b/server/resolv/async_resolv.c
@@ -442,6 +442,11 @@ ares_gethostbyname_wakeup(struct tevent_req *subreq)
state->family, resolv_gethostbyname_done, req);
}
+/* SRV and TXT parsing is not used anywhere in the code yet, so we disable it
+ * for now
+ */
+#ifdef BUILD_TXT_SRV
+
/*
* A simple helper function that will take an array of struct ares_srv_reply that
* was allocated by malloc() in c-ares and copies it using talloc. The old one
@@ -811,3 +816,4 @@ ares_gettxt_wakeup(struct tevent_req *subreq)
ns_c_in, ns_t_txt, resolv_gettxt_done, req);
}
+#endif
--
1.6.2.5
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel
