From: Vladimír Čunát <vladimir.cunat@nic.cz>
Date: Wed, 22 Feb 2017 12:50:10 +0000 (+0100)
Subject: modules/dnstap: nitpicks
X-Git-Tag: v1.3.0~23^2~62^2~4
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=429f47bcfe31067e377b8ca647e7b160d7ea6385;p=thirdparty%2Fknot-resolver.git

modules/dnstap: nitpicks
---

diff --git a/Makefile b/Makefile
index fb9114032..e4f5dbe61 100644
--- a/Makefile
+++ b/Makefile
@@ -75,7 +75,6 @@ endif
 
 # check for fstrm and protobuf for dnstap
 ifeq ($(HAS_libfstrm)|$(HAS_libprotobuf-c),yes|yes)
-BUILD_CFLAGS += -DENABLE_DNSTAP
 ENABLE_DNSTAP := yes
 endif
 
diff --git a/daemon/lua/kres-gen.lua b/daemon/lua/kres-gen.lua
index eca33cf76..309db5c9c 100644
--- a/daemon/lua/kres-gen.lua
+++ b/daemon/lua/kres-gen.lua
@@ -107,6 +107,7 @@ struct kr_request {
 		const struct sockaddr *dst_addr;
 		const knot_pkt_t *packet;
 		const knot_rrset_t *opt;
+		_Bool tcp;
 	} qsource;
 	struct {
 		unsigned int rtt;
diff --git a/doc/build.rst b/doc/build.rst
index d0b53a5bc..2b054b5dd 100644
--- a/doc/build.rst
+++ b/doc/build.rst
@@ -234,7 +234,8 @@ The project can be built with code coverage tracking using the ``COVERAGE=1`` va
 Running unit and integration tests
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-The unit tests require cmocka_ and are executed with ``make check``.
+The unit tests require cmocka_ and are executed by ``make check``.
+Tests for the dnstap module need go and are executed by ``make ckeck-dnstap``.
 
 The integration tests use Deckard, the `DNS test harness <deckard>`_.
 
diff --git a/lib/resolve.h b/lib/resolve.h
index c5c31cad2..b0dd2d01d 100644
--- a/lib/resolve.h
+++ b/lib/resolve.h
@@ -133,7 +133,7 @@ struct kr_request {
 		const struct sockaddr *dst_addr;
 		const knot_pkt_t *packet;
 		const knot_rrset_t *opt;
-		bool tcp; /* true if the query is on tcp */
+		bool tcp; /**< true if the request is on tcp; only meaningful if (dst_addr) */
 	} qsource;
 	struct {
 		unsigned rtt;                  /**< Current upstream RTT */
diff --git a/modules/dnstap/dnstap.c b/modules/dnstap/dnstap.c
index 54914b09a..872105ccf 100644
--- a/modules/dnstap/dnstap.c
+++ b/modules/dnstap/dnstap.c
@@ -102,10 +102,10 @@ static void set_address(const struct sockaddr *sockaddr,
 
 /* dnstap_log prepares dnstap message and sent it to fstrm */
 static int dnstap_log(kr_layer_t *ctx) {
-	struct kr_request *req = ctx->req;
-	struct kr_module *module = ctx->api->data;
-	struct kr_rplan *rplan = &req->rplan;
-	struct dnstap_data *dnstap_dt = module->data;
+	const struct kr_request *req = ctx->req;
+	const struct kr_module *module = ctx->api->data;
+	const struct kr_rplan *rplan = &req->rplan;
+	const struct dnstap_data *dnstap_dt = module->data;
 
 	/* check if we have a valid iothread */
 	if (!dnstap_dt->iothread || !dnstap_dt->ioq) {
@@ -126,13 +126,6 @@ static int dnstap_log(kr_layer_t *ctx) {
 	/* Only handling response */
 	m.type = DNSTAP__MESSAGE__TYPE__RESOLVER_RESPONSE;
 
-	if (req->qsource.tcp) {
-		m.socket_protocol = DNSTAP__SOCKET_PROTOCOL__TCP;
-	} else {
-		m.socket_protocol = DNSTAP__SOCKET_PROTOCOL__UDP;
-	}
-	m.has_socket_protocol = true;
-
 	if (req->qsource.addr) {
 		set_address(req->qsource.addr,
 				&m.query_address,
@@ -142,6 +135,13 @@ static int dnstap_log(kr_layer_t *ctx) {
 	}
 
 	if (req->qsource.dst_addr) {
+		if (req->qsource.tcp) {
+			m.socket_protocol = DNSTAP__SOCKET_PROTOCOL__TCP;
+		} else {
+			m.socket_protocol = DNSTAP__SOCKET_PROTOCOL__UDP;
+		}
+		m.has_socket_protocol = true;
+
 		set_address(req->qsource.dst_addr,
 				&m.response_address,
 				&m.has_response_address,
@@ -303,17 +303,6 @@ static int find_string(const JsonNode *node, char **val, size_t len) {
 	return kr_ok();
 }
 
-/* find_int copies json int into val
- * node must be of type JSON_NUMBER */
-static int find_int(const JsonNode *node, int *val) {
-	if (!node || !node->key || !val) {
-		return kr_error(EINVAL);
-	}
-	assert(node->tag == JSON_NUMBER);
-	*val = node->number_;
-	return kr_ok();
-}
-
 /* find_bool returns bool from json */
 static bool find_bool(const JsonNode *node) {
 	if (!node || !node->key) {
diff --git a/tests/dnstap/src/dnstap-test/dnstap.mk b/tests/dnstap/src/dnstap-test/dnstap.mk
index f6dce7184..0711707e1 100644
--- a/tests/dnstap/src/dnstap-test/dnstap.mk
+++ b/tests/dnstap/src/dnstap-test/dnstap.mk
@@ -6,12 +6,12 @@ CONFIG := $(DNSTAP_PATH)/config
 CMD := daemon/kresd
 ZONES := "fake1.localdomain,fake2.localdomain,fake3.localdomain"
 TIMEOUT := 60s
-check-dnstap:
+check-dnstap: daemon
 	@echo "Checking dnstap functionality"
 	GOPATH=$(GOPATH) go get -u github.com/FiloSottile/gvt
 	cd $(DNSTAP_PATH) && $(GOPATH)/bin/gvt restore
 	GOPATH=$(GOPATH) go install $(DNSTAP_TEST)
-	$(GOPATH)/bin/$(DNSTAP_TEST) -c $(CONFIG) -cmd $(CMD) -q $(ZONES) -t $(TIMEOUT) 
+	$(GOPATH)/bin/$(DNSTAP_TEST) -c $(CONFIG) -cmd $(CMD) -q $(ZONES) -t $(TIMEOUT)
 
 clean-dnstap:
 	rm -rf $(GOPATH)/bin $(GOPATH)/pkg
diff --git a/tests/tests.mk b/tests/tests.mk
index e85d92a08..12c6331b7 100644
--- a/tests/tests.mk
+++ b/tests/tests.mk
@@ -29,6 +29,6 @@ deckard: check-integration
 
 # Targets
 tests: check-unit
-tests-clean: $(foreach test,$(tests_BIN),$(test)-clean) mock_cmodule-clean
+tests-clean: $(foreach test,$(tests_BIN),$(test)-clean) mock_cmodule-clean clean-dnstap
 
 .PHONY: tests tests-clean check-integration deckard