Replace doubly-linked list with singly-linked list, eliminating edge cases. (8a560813) · Commits · kinetic-archive / kinetic-c

src/lib/kinetic_allocator.c

+28 −91

Original line number	Diff line number	Diff line
		@@ -94,14 +94,8 @@ static void* KineticAllocator_NewItem(KineticList* const list, size_t size)

		// Add the new item to the list
		KINETIC_LIST_LOCK(list);
		if (list->start == NULL) {
		newItem->next = list->start;
		list->start = newItem;
		}
		else {
		newItem->previous = list->last;
		list->last->next = newItem;
		}
		list->last = newItem;
		KINETIC_LIST_UNLOCK(list);

		LOGF3(" Allocated new list item (0x%0llX) w/data (0x%0llX)",
		@@ -117,6 +111,8 @@ static void KineticAllocator_FreeItem(KineticList* const list, void* item, bool
		KINETIC_LIST_LOCK(list);
		}
		KineticListItem* cur = list->start;
		KineticListItem* prev = NULL;

		while (cur->data != item) {
		if (cur->next == NULL) {
		LOG1(" Reached end of list before finding item to free!");
		@@ -126,6 +122,7 @@ static void KineticAllocator_FreeItem(KineticList* const list, void* item, bool
		return;
		}
		else {
		prev = cur;
		cur = cur->next;
		}
		}
		@@ -134,41 +131,12 @@ static void KineticAllocator_FreeItem(KineticList* const list, void* item, bool
		if ((cur != NULL) && (cur->data == item)) {
		LOG3(" item found! freeing it.");

		// Handle PDU list emptied
		if (cur->previous == NULL) {
		if (prev == NULL) {
		LOG3(" At start of list.");
		if (cur->next == NULL) {
		LOG3(" Making it empty, since all deallocated!");
		list->start = NULL;
		list->last = NULL;
		}
		else {
		LOG3(" Moving current item to head, since head deallocated!");
		list->start = cur->next;
		list->start->previous = NULL;
		}
		}
		else {
		// Relink from previous to next, if avaliable
		} else {
		LOG3(" Not at list start, so relinking list to free item.");
		if (cur->previous->next != NULL) {
		LOG3(" Relinking previous to next");
		if (cur->next != NULL) {
		LOG3(" Next being reset!");
		cur->previous->next = cur->next;
		cur->next->previous = cur->previous;
		}
		else {
		list->last = cur->previous;
		list->last->next = NULL;
		LOGF3(" Next is NULL. End of list now @ 0x%0llX",
		(long long)list->last);
		}
		}
		else {
		LOG1(" This shouldn't happen!");
		list->last = cur->previous;
		}
		prev->next = cur->next;
		}

		LOGF3(" Freeing item (0x%0llX) w/data (0x%0llX)", cur, &cur->data);
		@@ -187,33 +155,23 @@ static void KineticAllocator_FreeList(KineticList* const list)
		if (list != NULL) {
		LOGF3(" Freeing list (0x%0llX) of all items...", list);
		KINETIC_LIST_LOCK(list);
		KineticListItem* current = list->start;

		while (current->next != NULL) {
		LOG3(" Advancing to next list item...");
		current = current->next;
		}
		KineticListItem* next = NULL;
		for (KineticListItem* item = list->start; item; item = next) {
		next = item->next;

		while (current != NULL) {
		KineticListItem* curItem = current;
		KineticListItem* prevItem = current->previous;
		if (curItem != NULL) {
		LOGF3(" Freeing list item (0x%0llX) w/ data (0x%llX)",
		(long long)current,
		(long long)&current->data,
		(long long)current->previous);
		if (curItem->data != NULL) {
		free(curItem->data);
		(long long)item, (long long)&item->data);
		if (item->data != NULL) {
		free(item->data);
		item->data = NULL;
		}
		free(curItem);
		}
		current = prevItem;
		LOGF3(" on to previous list item (0x%llX)...", current);
		free(item);
		}

		// Make list empty, but leave mutex alone so the state is retained!
		list->start = NULL;
		list->last = NULL;
		/* list->last = NULL; */
		KINETIC_LIST_UNLOCK(list);
		}
		else {
		@@ -246,9 +204,10 @@ void KineticAllocator_FreePDU(KineticConnection* connection, KineticPDU* pdu)
		{
		LOGF3("Freeing PDU (0x%0llX) on connection (0x%0llX)", pdu, connection);
		KINETIC_LIST_LOCK(&connection->pdus);
		if ((pdu->proto != NULL) && pdu->protobufDynamicallyExtracted) {
		if (pdu && (pdu->proto != NULL) && pdu->protobufDynamicallyExtracted) {
		LOG3("Freeing dynamically allocated protobuf");
		KineticProto_Message__free_unpacked(pdu->proto, NULL);
		pdu->proto = NULL;
		};

		/* TODO: We can't unlock until the function below completes, but it
		@@ -327,11 +286,13 @@ void KineticAllocator_FreeOperation(KineticConnection* const connection, Kinetic
		LOGF3("Freeing request PDU (0x%0llX) from operation (0x%0llX) on connection (0x%0llX)",
		operation->request, operation, connection);
		KineticAllocator_FreePDU(connection, operation->request);
		operation->request = NULL;
		}
		if (operation->response != NULL) {
		LOGF3("Freeing response PDU (0x%0llX) from operation (0x%0llX) on connection (0x%0llX)",
		operation->response, operation, connection);
		KineticAllocator_FreePDU(connection, operation->response);
		operation->response = NULL;
		}
		pthread_mutex_destroy(&operation->timeoutTimeMutex);
		KineticAllocator_FreeItem(&connection->operations, (void*)operation, true);
		@@ -352,34 +313,10 @@ KineticOperation* KineticAllocator_GetNextOperation(KineticConnection* const con

		void KineticAllocator_FreeAllOperations(KineticConnection* const connection)
		{
		assert(connection != NULL);
		KineticListItem* current = connection->operations.start;
		if (current != NULL) {
		LOGF3("Freeing operations list (0x%0llX) from connection (0x%0llX)...",
		&connection->operations, connection);
		while (current != NULL) {
		KineticOperation* op = (KineticOperation*)current->data;
		if (op != NULL) {

		if (op->request != NULL) {
		LOGF3("Freeing request PDU (0x%0llX) from operation (0x%0llX) on connection (0x%0llX)",
		op->request, op, connection);
		KineticAllocator_FreePDU(connection, op->request);
		}

		if (op->response != NULL) {
		LOGF3("Freeing response PDU (0x%0llX) from op (0x%0llX) on connection (0x%0llX)",
		op->response, op, connection);
		KineticAllocator_FreePDU(connection, op->response);
		}

		current = current->next;
		KineticAllocator_FreeItem(&connection->operations, (void*)op, true);
		}
		}
		}
		else {
		LOG1(" Nothing to free!");
		KineticOperation* op = KineticAllocator_GetFirstOperation(connection);
		while (op) {
		KineticAllocator_FreeOperation(connection, op);
		op = KineticAllocator_GetFirstOperation(connection);
		}
		}

src/lib/kinetic_types_internal.h

+1 −3

Original line number	Diff line number	Diff line
		@@ -58,19 +58,17 @@ typedef struct _KineticConnection KineticConnection;
		typedef struct _KineticListItem KineticListItem;
		struct _KineticListItem {
		KineticListItem* next;
		KineticListItem* previous;
		void* data;
		};

		// Kinetic list
		typedef struct _KineticList {
		KineticListItem* start;
		KineticListItem* last;
		pthread_mutex_t mutex;
		bool locked;
		} KineticList;
		#define KINETIC_LIST_INITIALIZER (KineticList) { \
		.mutex = PTHREAD_MUTEX_INITIALIZER, .locked = false, .start = NULL, .last = NULL }
		.mutex = PTHREAD_MUTEX_INITIALIZER, .locked = false, .start = NULL, }

		// Kinetic Thread Instance
		typedef struct _KineticThread {

test/unit/test_kinetic_allocator.c

+18 −31

Original line number	Diff line number	Diff line
		@@ -42,12 +42,10 @@ void setUp(void)
		pthread_mutex_t expectedMutex = PTHREAD_MUTEX_INITIALIZER;

		TEST_ASSERT_NULL(Connection.operations.start);
		TEST_ASSERT_NULL(Connection.operations.last);
		TEST_ASSERT_EQUAL_MEMORY(&expectedMutex, &Connection.operations.mutex, sizeof(pthread_mutex_t));
		TEST_ASSERT_FALSE(Connection.operations.locked);

		TEST_ASSERT_NULL(Connection.pdus.start);
		TEST_ASSERT_NULL(Connection.pdus.last);
		TEST_ASSERT_EQUAL_MEMORY(&expectedMutex, &Connection.pdus.mutex, sizeof(pthread_mutex_t));
		TEST_ASSERT_FALSE(Connection.pdus.locked);
		}
		@@ -59,11 +57,9 @@ void tearDown(void)
		KineticAllocator_FreeAllOperations(&Connection);

		TEST_ASSERT_NULL(Connection.operations.start);
		TEST_ASSERT_NULL(Connection.operations.last);
		TEST_ASSERT_FALSE(Connection.operations.locked);

		TEST_ASSERT_NULL(Connection.pdus.start);
		TEST_ASSERT_NULL(Connection.pdus.last);
		TEST_ASSERT_FALSE(Connection.pdus.locked);

		TEST_ASSERT_TRUE_MESSAGE(allFreed, "Dynamically allocated things were not freed!");
		@@ -88,17 +84,16 @@ void test_KineticAllocator_GetFirstPDU_should_return_the_first_PDU_in_the_list(v

		TEST_ASSERT_NULL(KineticAllocator_GetFirstPDU(&Connection));

		pdus[0] = KineticAllocator_NewPDU(&Connection);
		TEST_ASSERT_EQUAL_PTR(pdus[0], KineticAllocator_GetFirstPDU(&Connection));

		pdus[1] = KineticAllocator_NewPDU(&Connection);
		TEST_ASSERT_EQUAL_PTR(pdus[1], KineticAllocator_GetFirstPDU(&Connection));

		pdus[0] = KineticAllocator_NewPDU(&Connection);
		TEST_ASSERT_EQUAL_PTR(pdus[0], KineticAllocator_GetFirstPDU(&Connection));

		KineticAllocator_FreeAllPDUs(&Connection);
		TEST_ASSERT_NULL(KineticAllocator_GetFirstPDU(&Connection));
		}


		void test_KineticAllocator_GetNextPDU_should_return_the_next_PDU_in_the_list(void)
		{
		LOG_LOCATION;
		@@ -106,15 +101,15 @@ void test_KineticAllocator_GetNextPDU_should_return_the_next_PDU_in_the_list(voi

		TEST_ASSERT_NULL(KineticAllocator_GetNextPDU(&Connection, NULL));

		pdus[0] = KineticAllocator_NewPDU(&Connection);
		pdus[2] = KineticAllocator_NewPDU(&Connection);
		TEST_ASSERT_NULL(KineticAllocator_GetNextPDU(&Connection, NULL));
		TEST_ASSERT_NULL(KineticAllocator_GetNextPDU(&Connection, pdus[0]));
		TEST_ASSERT_NULL(KineticAllocator_GetNextPDU(&Connection, pdus[2]));

		pdus[1] = KineticAllocator_NewPDU(&Connection);
		TEST_ASSERT_EQUAL_PTR(pdus[1], KineticAllocator_GetNextPDU(&Connection, pdus[0]));
		TEST_ASSERT_NULL(KineticAllocator_GetNextPDU(&Connection, pdus[1]));
		TEST_ASSERT_EQUAL_PTR(pdus[2], KineticAllocator_GetNextPDU(&Connection, pdus[1]));
		TEST_ASSERT_NULL(KineticAllocator_GetNextPDU(&Connection, pdus[2]));

		pdus[2] = KineticAllocator_NewPDU(&Connection);
		pdus[0] = KineticAllocator_NewPDU(&Connection);
		TEST_ASSERT_EQUAL_PTR(pdus[2], KineticAllocator_GetNextPDU(&Connection, pdus[1]));
		TEST_ASSERT_EQUAL_PTR(pdus[1], KineticAllocator_GetNextPDU(&Connection, pdus[0]));
		TEST_ASSERT_NULL(KineticAllocator_GetNextPDU(&Connection, pdus[2]));
		@@ -165,10 +160,7 @@ void test_KineticAllocator_NewPDU_should_allocate_new_PDUs_and_store_references(

		TEST_ASSERT_NOT_NULL(Connection.pdus.start);

		TEST_ASSERT_NULL(Connection.pdus.start->previous);
		TEST_ASSERT_NULL(Connection.pdus.start->next);
		TEST_ASSERT_NULL(Connection.pdus.last->next);
		TEST_ASSERT_NULL(Connection.pdus.last->previous);

		pdu = KineticAllocator_NewPDU(&Connection);
		TEST_ASSERT_NOT_NULL(pdu);
		@@ -274,15 +266,15 @@ void test_KineticAllocator_GetNextOperation_should_return_the_next_Operation_in_

		TEST_ASSERT_NULL(KineticAllocator_GetNextOperation(&Connection, NULL));

		operations[0] = KineticAllocator_NewOperation(&Connection);
		operations[2] = KineticAllocator_NewOperation(&Connection);
		TEST_ASSERT_NULL(KineticAllocator_GetNextOperation(&Connection, NULL));
		TEST_ASSERT_NULL(KineticAllocator_GetNextOperation(&Connection, operations[0]));
		TEST_ASSERT_NULL(KineticAllocator_GetNextOperation(&Connection, operations[2]));

		operations[1] = KineticAllocator_NewOperation(&Connection);
		TEST_ASSERT_EQUAL_PTR(operations[1], KineticAllocator_GetNextOperation(&Connection, operations[0]));
		TEST_ASSERT_NULL(KineticAllocator_GetNextOperation(&Connection, operations[1]));
		TEST_ASSERT_EQUAL_PTR(operations[2], KineticAllocator_GetNextOperation(&Connection, operations[1]));
		TEST_ASSERT_NULL(KineticAllocator_GetNextOperation(&Connection, operations[2]));

		operations[2] = KineticAllocator_NewOperation(&Connection);
		operations[0] = KineticAllocator_NewOperation(&Connection);
		TEST_ASSERT_EQUAL_PTR(operations[2], KineticAllocator_GetNextOperation(&Connection, operations[1]));
		TEST_ASSERT_EQUAL_PTR(operations[1], KineticAllocator_GetNextOperation(&Connection, operations[0]));
		TEST_ASSERT_NULL(KineticAllocator_GetNextOperation(&Connection, operations[2]));
		@@ -308,15 +300,13 @@ void test_KineticAllocator_FreeAllOperations_should_free_full_list_of_Operations
		{
		LOG_LOCATION;
		KINETIC_CONNECTION_INIT(&Connection);
		KineticOperation* operations[3];
		KineticOperation* operations[3] = {0, 0, 0};

		operations[0] = KineticAllocator_NewOperation(&Connection);
		operations[1] = KineticAllocator_NewOperation(&Connection);
		operations[2] = KineticAllocator_NewOperation(&Connection);
		TEST_ASSERT_FALSE(KineticAllocator_ValidateAllMemoryFreed(&Connection));
		operations[1] = KineticAllocator_NewOperation(&Connection);
		operations[0] = KineticAllocator_NewOperation(&Connection);

		KineticAllocator_FreePDU(&Connection, operations[0]->request);
		operations[1]->response = KineticAllocator_NewPDU(&Connection);
		TEST_ASSERT_FALSE(KineticAllocator_ValidateAllMemoryFreed(&Connection));

		KineticAllocator_FreeAllOperations(&Connection);
		TEST_ASSERT_TRUE(KineticAllocator_ValidateAllMemoryFreed(&Connection));
		@@ -331,10 +321,7 @@ void test_KineticAllocator_NewOperation_should_allocate_new_Operations_and_store
		TEST_ASSERT_NOT_NULL(operation);
		TEST_ASSERT_EQUAL_PTR(&Connection, operation->connection);
		TEST_ASSERT_NOT_NULL(Connection.operations.start);
		TEST_ASSERT_NULL(Connection.operations.start->previous);
		TEST_ASSERT_NULL(Connection.operations.start->next);
		TEST_ASSERT_NULL(Connection.operations.last->next);
		TEST_ASSERT_NULL(Connection.operations.last->previous);

		operation = KineticAllocator_NewOperation(&Connection);
		TEST_ASSERT_NOT_NULL(operation);
		@@ -347,7 +334,6 @@ void test_KineticAllocator_NewOperation_should_allocate_new_Operations_and_store
		KineticAllocator_FreeAllOperations(&Connection);
		}


		void test_KineticAllocator_should_allocate_and_free_a_single_Operation_list_item_with_request_PDU(void)
		{
		LOG_LOCATION;
		@@ -430,3 +416,4 @@ void test_KineticAllocator_should_allocate_and_free_multiple_Operation_list_item
		KineticAllocator_FreeOperation(&Connection, operations[2]);
		TEST_ASSERT_TRUE(KineticAllocator_ValidateAllMemoryFreed(&Connection));
		}