From 091e6bf4b9213884c8dc81510af9cb9da2cc8990 Mon Sep 17 00:00:00 2001
From: Johannes Schriewer <hallo@dunkelstern.de>
Date: Sun, 29 Jul 2018 03:41:55 +0200
Subject: [PATCH] General cleanup
---
.gitignore | 6 +++++
.vscode/settings.json | 4 ++++
Makefile.linux | 32 ++++++++++++++++++++-----
src/debug.h | 4 +++-
src/mqtt.h | 21 ++++++++++------
src/packet.c | 56 ++++++++++++++++++++++++++++++++++---------
tests/Makefile | 18 ++++++++------
tests/decode_packet.c | 23 ++++++++++++------
tests/encode_packet.c | 10 ++------
tests/test.h | 7 ++++++
10 files changed, 134 insertions(+), 47 deletions(-)
diff --git a/.gitignore b/.gitignore
index e0292b1..f6aa3a1 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1,2 +1,8 @@
*.o
*.a
+*.do
+*.gcno
+*.gcda
+*.gcov
+coverage/
+*.test
diff --git a/.vscode/settings.json b/.vscode/settings.json
index d086cad..937ca70 100644
--- a/.vscode/settings.json
+++ b/.vscode/settings.json
@@ -22,6 +22,10 @@
"**/CVS": true,
"**/.DS_Store": true,
"**/*.o": true,
+ "**/*.do": true,
+ "**/*.gcno": true,
+ "**/*.gcda": true,
+ "**/*.gcov": true,
"**/*.a": true
}
}
diff --git a/Makefile.linux b/Makefile.linux
index 15eefab..0bd782a 100644
--- a/Makefile.linux
+++ b/Makefile.linux
@@ -1,22 +1,39 @@
SRCS=src/mqtt.c src/packet.c src/protocol.c src/debug.c platform/linux.c
OBJS=$(SRCS:%.c=%.o)
+DEBUG_OBJS=$(SRCS:%.c=%.do)
+COVERAGE_FILES=$(SRCS:%.c=%.gcno) $(SRCS:%.c=%.gcda)
TARGET=libmqtt.a
+DEBUG_TARGET=libmqtt-debug.a
-PLATFORM_FLAGS=-DMAX_BUFFER_SIZE=256 -DDEBUG=1
+PLATFORM_FLAGS=
AR=ar
-CC=clang
+CC=gcc
CFLAGS=-g -Os -Wall -pthread -I./platform -I./src $(PLATFORM_FLAGS)
+COVERAGE_FLAGS=-fprofile-arcs -ftest-coverage
+DEBUG_FLAGS=-DDEBUG=1 $(COVERAGE_FLAGS)
-all: $(TARGET)
+all: $(TARGET) $(DEBUG_TARGET)
-test: $(TARGET)
+test: $(DEBUG_TARGET)
$(MAKE) -C tests
+coverage: test
+ lcov --capture --directory . --output-file lcov.info
+ rm -rf coverage
+ mkdir -p coverage
+ genhtml -o coverage lcov.info
+
$(TARGET): $(OBJS)
$(AR) -cr $(TARGET) $(OBJS)
+$(DEBUG_TARGET): $(DEBUG_OBJS)
+ $(AR) -cr $(DEBUG_TARGET) $(DEBUG_OBJS)
+
+%.do: %.c
+ $(CC) $(CFLAGS) $(DEBUG_FLAGS) -o $@ -c $<
+
%.o: %.c
$(CC) $(CFLAGS) -o $@ -c $<
@@ -24,6 +41,9 @@ mqtt.o: mqtt.h
clean:
$(MAKE) -C tests clean
- rm -f $(TARGET)
- rm -f $(OBJS)
+ rm -f $(TARGET) $(DEBUG_TARGET)
+ rm -f $(OBJS) $(DEBUG_OBJS)
+ rm -f $(COVERAGE_FILES)
+ rm -f *.gcov
+ rm -f lcov.info
rm -rf docs/
diff --git a/src/debug.h b/src/debug.h
index edbb7bb..bf0c477 100644
--- a/src/debug.h
+++ b/src/debug.h
@@ -6,8 +6,10 @@
#include <stdlib.h>
#include <stdio.h>
#include <stdint.h>
+#include <string.h>
+#include <errno.h>
-#define DEBUG_LOG(fmt, ...) fprintf(stderr, fmt "\n", ## __VA_ARGS__);
+#define DEBUG_LOG(fmt, ...) fprintf(stderr, "%s:%d: " fmt "\n", __FILE__, __LINE__, ## __VA_ARGS__);
static inline void hexdump(char *data, size_t len, int indent) {
for (int i = 0; i < len;) {
diff --git a/src/mqtt.h b/src/mqtt.h
index eadbfe8..51e536c 100644
--- a/src/mqtt.h
+++ b/src/mqtt.h
@@ -7,13 +7,13 @@
typedef struct _MQTTHandle MQTTHandle;
typedef struct {
- char *hostname; /**< Hostname to connect to, will do DNS resolution */
- uint16_t port; /**< Port the broker listens on, set to 0 for 1883 default */
+ char *hostname; /**< Hostname to connect to, will do DNS resolution */
+ uint16_t port; /**< Port the broker listens on, set to 0 for 1883 default */
- char *clientID; /**< Client identification */
+ char *client_id; /**< Client identification */
- char *username; /**< User name, set to NULL to connect anonymously */
- char *password; /**< Password, set to NULL to connect without password */
+ char *username; /**< User name, set to NULL to connect anonymously */
+ char *password; /**< Password, set to NULL to connect without password */
} MQTTConfig;
typedef enum {
@@ -36,8 +36,11 @@ typedef enum {
MQTT_Error_Connection_Reset /**< Network connection reset, perhaps network went down? */
} MQTTErrorCode;
-/** Error handler callback */
-typedef void (*MQTTErrorHandler)(MQTTHandle *handle, MQTTErrorCode code);
+/** Error handler callback
+ *
+ * Return true if the handle should be freed, false to keep it
+ */
+typedef bool (*MQTTErrorHandler)(MQTTHandle *handle, MQTTErrorCode code);
/** Event handler callback */
typedef void (*MQTTEventHandler)(MQTTHandle *handle, char *topic, char *payload);
@@ -48,6 +51,10 @@ typedef void (*MQTTEventHandler)(MQTTHandle *handle, char *topic, char *payload)
* @param config: MQTT configuration
* @param callback: Callback function to call on errors
* @returns handle to mqtt connection or NULL on error
+ *
+ * If the error handler is called with Host not found or Connection refused,
+ * the handler is in charge of freeing the handle by returning true
+ * or re-trying by changing settings and calling mqtt_reconnect() and returning false
*/
MQTTHandle *mqtt_connect(MQTTConfig *config, MQTTErrorHandler callback);
diff --git a/src/packet.c b/src/packet.c
index 76bc4bb..e27f660 100644
--- a/src/packet.c
+++ b/src/packet.c
@@ -168,7 +168,10 @@ bool decode_connect(Buffer *buffer, ConnectPayload *payload) {
payload->protocol_level = buffer->data[buffer->position++];
uint8_t flags = buffer->data[buffer->position++];
payload->clean_session = ((flags & 0x02) > 0);
- payload->keepalive_interval = (buffer->data[buffer->position++] << 8) + buffer->data[buffer->position++];
+ payload->keepalive_interval =
+ (buffer->data[buffer->position] << 8)
+ + buffer->data[buffer->position + 1];
+ buffer->position += 2;
payload->client_id = utf8_string_decode(buffer);
// last will
@@ -208,7 +211,10 @@ bool decode_publish(Buffer *buffer, PublishPayload *payload, size_t sz) {
payload->duplicate = ((flags & 0x08) > 0);
payload->topic = utf8_string_decode(buffer);
- payload->packet_id = (buffer->data[buffer->position++] << 8) + buffer->data[buffer->position++];
+ payload->packet_id =
+ (buffer->data[buffer->position] << 8)
+ + buffer->data[buffer->position + 1];
+ buffer->position += 2;
size_t len = sz - (buffer->position - start_pos) + 1;
if (len > 1) {
@@ -222,31 +228,46 @@ bool decode_publish(Buffer *buffer, PublishPayload *payload, size_t sz) {
}
bool decode_puback(Buffer *buffer, PubAckPayload *payload) {
- payload->packet_id = (buffer->data[buffer->position++] << 8) + buffer->data[buffer->position++];
-
+ payload->packet_id =
+ (buffer->data[buffer->position] << 8)
+ + buffer->data[buffer->position + 1];
+ buffer->position += 2;
return true;
}
bool decode_pubrec(Buffer *buffer, PubRecPayload *payload) {
- payload->packet_id = (buffer->data[buffer->position++] << 8) + buffer->data[buffer->position++];
+ payload->packet_id =
+ (buffer->data[buffer->position] << 8)
+ + buffer->data[buffer->position + 1];
+ buffer->position += 2;
return true;
}
bool decode_pubrel(Buffer *buffer, PubRelPayload *payload) {
- payload->packet_id = (buffer->data[buffer->position++] << 8) + buffer->data[buffer->position++];
+ payload->packet_id =
+ (buffer->data[buffer->position] << 8)
+ + buffer->data[buffer->position + 1];
+ buffer->position += 2;
return true;
}
bool decode_pubcomp(Buffer *buffer, PubCompPayload *payload) {
- payload->packet_id = (buffer->data[buffer->position++] << 8) + buffer->data[buffer->position++];
+ payload->packet_id =
+ (buffer->data[buffer->position] << 8)
+ + buffer->data[buffer->position + 1];
+ buffer->position += 2;
return true;
}
bool decode_subscribe(Buffer *buffer, SubscribePayload *payload) {
- payload->packet_id = (buffer->data[buffer->position++] << 8) + buffer->data[buffer->position++];
+ payload->packet_id =
+ (buffer->data[buffer->position] << 8)
+ + buffer->data[buffer->position + 1];
+ buffer->position += 2;
+
payload->topic = utf8_string_decode(buffer);
payload->qos = buffer->data[buffer->position++] & 0x03;
@@ -254,21 +275,32 @@ bool decode_subscribe(Buffer *buffer, SubscribePayload *payload) {
}
bool decode_suback(Buffer *buffer, SubAckPayload *payload) {
- payload->packet_id = (buffer->data[buffer->position++] << 8) + buffer->data[buffer->position++];
+ payload->packet_id =
+ (buffer->data[buffer->position] << 8)
+ + buffer->data[buffer->position + 1];
+ buffer->position += 2;
+
payload->status = buffer->data[buffer->position++];
return true;
}
bool decode_unsubscribe(Buffer *buffer, UnsubscribePayload *payload) {
- payload->packet_id = (buffer->data[buffer->position++] << 8) + buffer->data[buffer->position++];
+ payload->packet_id =
+ (buffer->data[buffer->position] << 8)
+ + buffer->data[buffer->position + 1];
+ buffer->position += 2;
+
payload->topic = utf8_string_decode(buffer);
return true;
}
bool decode_unsuback(Buffer *buffer, UnsubAckPayload *payload) {
- payload->packet_id = (buffer->data[buffer->position++] << 8) + buffer->data[buffer->position++];
+ payload->packet_id =
+ (buffer->data[buffer->position] << 8)
+ + buffer->data[buffer->position + 1];
+ buffer->position += 2;
return true;
}
@@ -614,4 +646,6 @@ Buffer *mqtt_packet_encode(MQTTPacket *packet) {
case PacketTypeDisconnect:
return encode_disconnect();
}
+
+ return NULL;
}
diff --git a/tests/Makefile b/tests/Makefile
index 5e3995d..fe3fe13 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -1,19 +1,22 @@
-SRCS=encode_packet.c decode_packet.c
+SRCS=encode_packet.c decode_packet.c connect_publish.c connect_subscribe.c
OBJS=$(SRCS:%.c=%.o)
+COVERAGE_FILES=$(SRCS:%.c=%.gcno) $(SRCS:%.c=%.gcda)
TARGETS=$(SRCS:%.c=%.test)
-CC=clang
-CFLAGS=-g -Os -Wall -I.. -I../src -I../platform -DDEBUG=1
+COVERAGE_FLAGS=-fprofile-arcs -ftest-coverage
+
+CC=gcc
+CFLAGS=-g -Os -Wall -I.. -I../src -I../platform -DDEBUG=1 $(COVERAGE_FLAGS)
# -DTIMETRIAL
LDFLAGS=
-LIBS=-L.. -lmqtt
+LIBS=-L.. -lmqtt-debug -lpthread
all: $(TARGETS)
%.test: %.o cputime.o
- $(CC) $(LDFLAGS) -o $@ cputime.o $< $(LIBS)
- ./$@
- rm $@
+ $(CC) $(COVERAGE_FLAGS) $(LDFLAGS) -o $@ cputime.o $< $(LIBS)
+ # ./$@
+ # rm $@
%.o: %.c test.h
$(CC) $(CFLAGS) -o $@ -c $<
@@ -26,4 +29,5 @@ all: $(TARGETS)
clean:
rm -f $(TARGETS)
rm -f $(OBJS)
+ rm -f $(COVERAGE_FILES)
rm -f *.e
diff --git a/tests/decode_packet.c b/tests/decode_packet.c
index e5701a2..142a06b 100644
--- a/tests/decode_packet.c
+++ b/tests/decode_packet.c
@@ -101,6 +101,21 @@ TestResult test_decode_connect_simple(void) {
TEST_OK();
}
+TestResult test_decode_connect_invalid(void) {
+ char data[] = {
+ 0x10, 0x10, // header
+ 0x00, 0x04, 'M', 'Q', 'T', 'X', 0x04, 0x02, 0x00, 0x0a, // var header
+ 0x00, 0x04, 't', 'e', 's', 't' // client id
+ };
+ Buffer *buffer = buffer_from_data_copy(data, sizeof(data));
+ MQTTPacket *packet = mqtt_packet_decode(buffer);
+
+ TESTASSERT(packet == NULL, "Packet should not be valid");
+ buffer_release(buffer);
+
+ TEST_OK();
+}
+
TestResult test_decode_connect_will(void) {
char data[] = {
0x10, 0x2d, // header
@@ -458,13 +473,6 @@ TestResult test_decode_disconnect(void) {
TEST_OK();
}
-// not implemented placeholder
-
-TestResult not_implemented(void) {
- TESTRESULT(TestStatusSkipped, "Not implemented");
-}
-
-
TESTS(
TEST("Variable length int decode for 0", test_vl_int_data_0),
TEST("Variable length int decode for 127", test_vl_int_data_127),
@@ -475,6 +483,7 @@ TESTS(
TEST("UTF-8 string decode empty string", test_utf8_string_empty),
TEST("UTF-8 string decode \"hello\"", test_utf8_string_hello),
TEST("Decode Connect simple", test_decode_connect_simple),
+ TEST("Decode Connect invalid", test_decode_connect_invalid),
TEST("Decode Connect with will", test_decode_connect_will),
TEST("Decode Connect with auth", test_decode_connect_auth),
TEST("Decode ConnAck", test_decode_connack),
diff --git a/tests/encode_packet.c b/tests/encode_packet.c
index c1ed28d..fbae50d 100644
--- a/tests/encode_packet.c
+++ b/tests/encode_packet.c
@@ -276,7 +276,7 @@ TestResult test_encode_publish_no_msg(void) {
TestResult test_encode_publish_with_msg(void) {
char data[] = {
- 0x33, 0x15, // header, qos1, retain
+ 0x3b, 0x15, // header, qos1, retain
0x00, 0x0a, 't', 'e', 's', 't', '/', 't', 'o', 'p', 'i', 'c',
0x00, 0x0a, // packet id
'p', 'a', 'y', 'l', 'o', 'a', 'd'
@@ -285,6 +285,7 @@ TestResult test_encode_publish_with_msg(void) {
payload->qos = MQTT_QOS_1;
payload->retain = true;
+ payload->duplicate = true;
payload->topic = "test/topic";
payload->packet_id = 10;
payload->message = "payload";
@@ -489,13 +490,6 @@ TestResult test_encode_disconnect(void) {
);
}
-// not implemented placeholder
-
-TestResult not_implemented(void) {
- TESTRESULT(TestStatusSkipped, "Not implemented");
-}
-
-
TESTS(
TEST("Variable length int size for 0", test_vl_int_0),
TEST("Variable length int size for 127", test_vl_int_127),
diff --git a/tests/test.h b/tests/test.h
index 4e42d51..034fc20 100644
--- a/tests/test.h
+++ b/tests/test.h
@@ -64,6 +64,13 @@ static inline TestResult TESTMEMCMP(Buffer *template, Buffer *check) {
}
}
+// not implemented placeholder
+
+static TestResult not_implemented(void) {
+ TESTRESULT(TestStatusSkipped, "Not implemented");
+}
+
+
void timetrial(DefinedTest *test);
int main(int argc, char **argv) {