On Mon, Jun 27, 2011 at 12:56 PM, Andrew Beekhof <andrew(a)beekhof.net> wrote:
Can you please explain the need for "union {...} answer" ?
You don't appear to use the HEADER field anywhere.
Pressed send too soon.
Re: mh_srv_lookup(), why not have it return a dynamic buffer?
You're only returning a failed error code which would be implied by return NULL;
On Tue, Jun 21, 2011 at 12:29 AM, Adam Stokes <astokes(a)fedoraproject.org> wrote:
> - Set buffer lengths to remove need for strdup or memory leaks :\
> - Use helper routine ns_name_uncompress to pull the hostname from
> srv record and to make the code easier to understand.
> - Add doxygen comment
> - Updated test case.
>
> Signed-off-by: Adam Stokes <astokes(a)fedoraproject.org>
> ---
> src/include/matahari/dnssrv.h | 30 ++++++++++
> src/lib/dnssrv_linux.c | 126 ++++++++++++++++-------------------------
> src/lib/dnssrv_private.h | 42 --------------
> src/lib/mh_agent.cpp | 18 ++++++-
> src/tests/mh_hsa.h | 59 +++++++++++++++++++
> 5 files changed, 156 insertions(+), 119 deletions(-)
> create mode 100644 src/include/matahari/dnssrv.h
> delete mode 100644 src/lib/dnssrv_private.h
> create mode 100644 src/tests/mh_hsa.h
>
> diff --git a/src/include/matahari/dnssrv.h b/src/include/matahari/dnssrv.h
> new file mode 100644
> index 0000000..abf604a
> --- /dev/null
> +++ b/src/include/matahari/dnssrv.h
> @@ -0,0 +1,30 @@
> +/* dnssrv.h - Copyright (c) 2011 Red Hat, Inc.
> + * Written by Adam Stokes <astokes(a)fedoraproject.org>
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Library General Public
> + * License as published by the Free Software Foundation; either
> + * version 2 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Library General Public License for more details.
> + *
> + * You should have received a copy of the GNU Library General Public
> + * License along with this library; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +/**
> + * \file
> + * \brief DNS SRV Lookup
> + * \ingroup coreapi
> + */
> +
> +#ifndef __MH_DNSSRV_H__
> +#define __MH_DNSSRV_H__
> +
> +extern int mh_srv_lookup(const char *query, char *target);
> +
> +#endif /* __MH_DNSSRV_H__ */
> diff --git a/src/lib/dnssrv_linux.c b/src/lib/dnssrv_linux.c
> index c1e11d8..d4f8fc5 100644
> --- a/src/lib/dnssrv_linux.c
> +++ b/src/lib/dnssrv_linux.c
> @@ -16,95 +16,69 @@
> * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> */
>
> +#include <string.h>
> +#include <errno.h>
> +#include <glib.h>
> +#include <stdlib.h>
> +
> #include <netinet/in.h>
> #include <arpa/nameser.h>
> #include <resolv.h>
> -#include <glib.h>
>
> -#include "dnssrv_private.h"
> +#include "matahari/dnssrv.h"
> +
> +/**
> + * Domain lookup providing a Matahari broker
> + *
> + * \param[in] srv record query i.e. "_matahari._tcp.matahariproject.org
> + * \param[in] set buffer to hold domain retrieved
> + *
> + * \return 0 or greater for successful match
> + */
>
> -struct srv_reply **srv_lookup(char *service,
> - char *protocol,
> - char *domain)
> +int
> +mh_srv_lookup(const char *query, char *target)
> {
> - struct srv_reply **replies = NULL;
> -#ifdef HAVE_RESOLV_A
> - struct srv_reply *reply = NULL;
> - char name[MAX_NAME_LEN];
> - unsigned char querybuf[MAX_NAME_LEN];
> - const unsigned char *buf;
> + union {
> + HEADER hdr;
> + unsigned char buf[NS_PACKETSZ];
> + } answer;
> ns_msg nsh;
> ns_rr rr;
> - int, i, n, len, size;
> -
> - g_snprintf(name, sizeof(name), "_%s._%s.%s", service, protocol,
domain);
> - if((size = res_query(name, C_IN, T_SRV, querybuf, sizeof(querybuf))) < 0) {
> - return NULL;
> - }
> -
> - if(ns_initparse(querybuf, size, &nsh) != 0) {
> - reutnr NULL;
> - }
> -
> - n = 0;
> - while(ns_parserr(&nsh, ns_s_an, n, &rr) == 0) {
> - size = ns_rr_rdlen(rr);
> - buf = ns_rr_rdata(rr);
> -
> - len = 0;
> - for(i=6; i < size && buf[i]; i+= buf[i] + 1) {
> - len += buf[i] + 1;
> - }
> + int size, rrnum;
>
> - if(i > size) {
> - break;
> + size = res_query(query,
> + C_IN,
> + T_SRV,
> + (u_char *)&answer,
> + sizeof(answer));
> + if (size > 0) {
> + if (ns_initparse(answer.buf, size, &nsh) < 0) {
> + goto fail;
> }
> + }
>
> - reply = g_malloc(sizeof(struct srv_reply) + len);
> - memcpy(reply->name, buf+7, len);
> -
> - for(i=buf[6]; i < len && buf[7+i]; i += buf[7+i] + 1) {
> - reply->name[i] = '.';
> + for (rrnum = 0; rrnum < ns_msg_count(nsh, ns_s_an); rrnum++) {
> + if (ns_parserr(&nsh, ns_s_an, rrnum, &rr)) {
> + goto fail;
> }
>
> - if(i > len) {
> - g_free(reply);
> - break;
> + if (ns_rr_type(rr) == T_SRV) {
> + char buf[NS_MAXDNAME];
> + /* Only care about domain name from rdata
> + * First 6 elements in rdata are broken up
> + * contain dns information such as type, class
> + * ttl, rdlength.
> + */
> + ns_name_uncompress(ns_msg_base(nsh),
> + ns_msg_end(nsh),
> + ns_rr_rdata(rr)+6,
> + buf,
> + sizeof(buf));
> + strcpy(target, buf);
> }
> -
> - reply->prio = (buf[0] << 8) | buf[1];
> - reply->weight = (buf[2] << 8) | buf[3];
> - reply->port = (buf[4] << 8) | buf[5];
> - n++;
> - replies = g_renew(struct srv_reply *, replies, n+1);
> - replies[n-1] = reply;
> - }
> - if(replies) {
> - replies[n] = NULL;
> - }
> -#endif
> - return replies;
> -}
> -
> -void srv_free(struct srv_reply **srv)
> -{
> - int i;
> - if(srv == NULL) {
> - return;
> - }
> -
> - for(i = 0; srv[i]; i++) {
> - g_free(srv[i]);
> }
> - g_free(srv);
> + return 0;
> +fail:
> + return -1;
> }
> -
> -/*
> - * Local Variables:
> - * mode: c
> - * c-basic-offset: 4
> - * indent-tabs-mode: nil
> - * End:
> - * vim: et
> - */
> -
> diff --git a/src/lib/dnssrv_private.h b/src/lib/dnssrv_private.h
> deleted file mode 100644
> index 0752311..0000000
> --- a/src/lib/dnssrv_private.h
> +++ /dev/null
> @@ -1,42 +0,0 @@
> -/* dnssrv.h - Copyright (c) 2011 Red Hat, Inc.
> - * Written by Adam Stokes <astokes(a)fedoraproject.org>
> - *
> - * This library is free software; you can redistribute it and/or
> - * modify it under the terms of the GNU Library General Public
> - * License as published by the Free Software Foundation; either
> - * version 2 of the License, or (at your option) any later version.
> - *
> - * This library is distributed in the hope that it will be useful,
> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> - * Library General Public License for more details.
> - *
> - * You should have received a copy of the GNU Library General Public
> - * License along with this library; if not, write to the Free Software
> - * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> - */
> -
> -#define MAX_NAME_LEN 1024
> -
> -struct srv_reply
> -{
> - int prio;
> - int weight;
> - int port;
> - char name[];
> -};
> -
> -struct srv_reply **srv_lookup(char *service,
> - char *protocol,
> - char *domain);
> -
> -void srv_free(struct srv_reply **srv);
> -
> -/*
> - * Local Variables:
> - * mode: c
> - * c-basic-offset: 4
> - * indent-tabs-mode: nil
> - * End:
> - * vim: et
> - */
> diff --git a/src/lib/mh_agent.cpp b/src/lib/mh_agent.cpp
> index a09c970..9b87bf3 100644
> --- a/src/lib/mh_agent.cpp
> +++ b/src/lib/mh_agent.cpp
> @@ -43,6 +43,7 @@ int use_stderr = 0;
>
> extern "C" {
> #include "matahari/logging.h"
> +#include "matahari/dnssrv.h"
> }
>
> using namespace qpid::management;
> @@ -351,7 +352,22 @@ mh_parse_options(const char *proc_name, int argc, char **argv,
qpid::types::Vari
> protocol = "tcp";
> }
>
> - if(servername == NULL) {
> + /*
> + * Based on servername do a SRV lookup for best possible
> + * server providing this service.
> + */
> + if (servername) {
> +#ifdef HAVE_RESOLV_A
> + int ret;
> + char query[NS_MAXDNAME];
> + char target[NS_MAXDNAME];
> + g_snprintf(query, NS_MAXDNAME, "_matahari._tcp.%s", servername);
> + ret = mh_srv_lookup(query, target);
> + if (ret == 0) {
> + servername = strdup(target);
> + }
> +#endif
> + } else {
> servername = strdup(MATAHARI_BROKER);
> }
>
> diff --git a/src/tests/mh_hsa.h b/src/tests/mh_hsa.h
> new file mode 100644
> index 0000000..286526a
> --- /dev/null
> +++ b/src/tests/mh_hsa.h
> @@ -0,0 +1,59 @@
> +/*
> + * mh_hs.h: dns srv high availability test
> + *
> + * Copyright (C) 2011 Red Hat Inc.
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
> + *
> + * Author: Adam Stokes <astokes(a)fedoraproject.org>
> + */
> +
> +
> +#ifndef __MH_HSA_UNITTEST_H
> +#define __MH_HSA_UNITTEST_H
> +
> +#include <string.h>
> +#include <stdlib.h>
> +#include <cxxtest/TestSuite.h>
> +
> +extern "C" {
> +#include <netinet/in.h>
> +#include <arpa/nameser.h>
> +#include <resolv.h>
> +#include "mh_test_utilities.h"
> +#include "matahari/dnssrv.h"
> +};
> +
> +using namespace std;
> +
> +class MhHsaSuite : public CxxTest::TestSuite
> +{
> + public:
> + std::stringstream infomsg;
> +
> + void testSrvLookup(void)
> + {
> + char target[NS_MAXDNAME];
> + const char *host = "_matahari._tcp.matahariproject.org";
> + int ret = 0;
> +
> + ret = mh_srv_lookup(host, target);
> + TS_ASSERT(ret == 0);
> + TS_ASSERT((mh_test_is_match("^qpid.*matahariproject\\.org$",
> + target)) >= 0);
> + }
> +};
> +
> +#endif
> --
> 1.7.5.4
>
> _______________________________________________
> Matahari mailing list
> Matahari(a)lists.fedorahosted.org
>
https://fedorahosted.org/mailman/listinfo/matahari
>