General cleanup

.gitignore

@@ -1,2 +1,8 @@
 *.o
 *.a
+*.do
+*.gcno
+*.gcda
+*.gcov
+coverage/
+*.test

.vscode/settings.json

@@ -22,6 +22,10 @@
         "**/CVS": true,
         "**/.DS_Store": true,
         "**/*.o": true,
+        "**/*.do": true,
+        "**/*.gcno": true,
+        "**/*.gcda": true,
+        "**/*.gcov": true,
         "**/*.a": true
     }
 }

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/

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;) {

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);
 

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;
 }

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

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),

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),

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) {