From mboxrd@z Thu Jan  1 00:00:00 1970
From: guido@trentalancia.com (Guido Trentalancia)
Date: Sat, 30 Sep 2017 08:03:43 +0200
Subject: [refpolicy] [PATCH v4] fc_sort: memory leakages
In-Reply-To: <1506727850.25420.6.camel@trentalancia.com>
References: <CAFftDdr6uZxWz9FPBnP4RW0Y-KLcS7aFTq0hHaF_dNB6K8wi2g@mail.gmail.com>
	<1506643940.32317.6.camel@trentalancia.com>
	<CAFftDdqy_BxjsPZjqqPFQZxhVA8sHG1Fq1adfCgjKAkuBsNk-w@mail.gmail.com>
	<1506703966.24492.1.camel@trentalancia.com>
	<1506710145.25223.1.camel@trentalancia.com>
	<1506727850.25420.6.camel@trentalancia.com>
Message-ID: <1506751423.18819.1.camel@trentalancia.com>
To: refpolicy@oss.tresys.com
List-Id: refpolicy.oss.tresys.com

Avoid memory leakages in the fc_sort executable (now passes
all valgrind tests fine).

Some NULL pointer checks with or without associated error
reporting.

Some white space and comment formatting fixes.

Optimization: avoid unnecessary operations (unnecessary
memory allocation/deallocation and list copying).

Reverts 7821eb6f37d785ab6ac3bbdc39282c799ad22393 as there
are good chances that it is no longer needed, given that
all memory leakages should now be fixed.

This is the fourth version of this patch. Please do not use
the first version as it introduces a serious bug.

Signed-off-by: Guido Trentalancia <guido@trentalancia.com>
---
 support/fc_sort.c |  102 +++++++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 78 insertions(+), 24 deletions(-)

--- a/support/fc_sort.c	2017-09-30 02:44:18.137342226 +0200
+++ b/support/fc_sort.c	2017-09-30 07:47:46.141267786 +0200
@@ -46,6 +46,9 @@ typedef struct file_context_node {
 
 void file_context_node_destroy(file_context_node_t *x)
 {
+	if (!x)
+		return;
+
 	free(x->path);
 	free(x->file_type);
 	free(x->context);
@@ -307,6 +310,41 @@ void fc_fill_data(file_context_node_t *f
 	}
 }
 
+
+
+/* fc_free_file_context_node_list
+ * Free the memory allocated to the linked list and its elements.
+ */
+void fc_free_file_context_node_list(struct file_context_node *node)
+{
+	struct file_context_node *next;
+
+	while (node) {
+		next = node->next;
+		file_context_node_destroy(node);
+		free(node);
+		node = next;
+	}
+}
+
+
+
+/* fc_free_file_context_bucket_list
+ * Free the memory allocated to the linked list and its elements.
+ */
+void fc_free_file_context_bucket_list(struct file_context_bucket *element)
+{
+	struct file_context_bucket *next;
+
+	while (element) {
+		next = element->next;
+		file_context_node_destroy(element->data);
+		free(element->data);
+		free(element);
+		element = next;
+	}
+}
+
 /* main
  * This program takes in two arguments, the input filename and the
  *  output filename. The input file should be syntactically correct.
@@ -328,7 +366,6 @@ int main(int argc, char *argv[])
 
 	FILE *in_file, *out_file;
 
-
 	/* Check for the correct number of command line arguments. */
 	if (argc < 2 || argc > 3) {
 		fprintf(stderr, "Usage: %s <infile> [<outfile>]\n",argv[0]);
@@ -348,24 +385,33 @@ int main(int argc, char *argv[])
 
 	/* Initialize the head of the linked list. */
 	head = current = (file_context_node_t*)malloc(sizeof(file_context_node_t));
+	if (!head) {
+		fprintf(stderr, "Error: failure allocating memory.\n");
+		return 1;
+	}
 	head->next = NULL;
+	head->path = NULL;
+	head->file_type = NULL;
+	head->context = NULL;
 
 	/* Parse the file into a file_context linked list. */
 	line_buf = NULL;
 
 	while ( getline(&line_buf, &buf_len, in_file) != -1 ){
 		line_len = strlen(line_buf);
+
 		if( line_len == 0 || line_len == 1)
 			continue;
+
 		/* Get rid of whitespace from the front of the line. */
 		for (i = 0; i < line_len; i++) {
 			if (!isspace(line_buf[i]))
 				break;
 		}
 
-
 		if (i >= line_len)
 			continue;
+
 		/* Check if the line isn't empty and isn't a comment */
 		if (line_buf[i] == '#')
 			continue;
@@ -373,7 +419,9 @@ int main(int argc, char *argv[])
 		/* We have a valid line - allocate a new node. */
 		temp = (file_context_node_t *)malloc(sizeof(file_context_node_t));
 		if (!temp) {
+			free(line_buf);
 			fprintf(stderr, "Error: failure allocating memory.\n");
+			fc_free_file_context_node_list(head);
 			return 1;
 		}
 		temp->next = NULL;
@@ -382,19 +430,15 @@ int main(int argc, char *argv[])
 		/* Parse out the regular expression from the line. */
 		start = i;
 
-
 		while (i < line_len && (!isspace(line_buf[i])))
 			i++;
 		finish = i;
 
-
 		regex_len = finish - start;
 
 		if (regex_len == 0) {
 			file_context_node_destroy(temp);
 			free(temp);
-
-
 			continue;
 		}
 
@@ -402,13 +446,14 @@ int main(int argc, char *argv[])
 		if (!temp->path) {
 			file_context_node_destroy(temp);
 			free(temp);
+			free(line_buf);
 			fprintf(stderr, "Error: failure allocating memory.\n");
+			fc_free_file_context_node_list(head);
 			return 1;
 		}
 
 		/* Get rid of whitespace after the regular expression. */
 		for (; i < line_len; i++) {
-
 			if (!isspace(line_buf[i]))
 				break;
 		}
@@ -420,18 +465,21 @@ int main(int argc, char *argv[])
 		}
 
 		/* Parse out the type from the line (if it
-			*  is there). */
+		 *  is there). */
 		if (line_buf[i] == '-') {
 			temp->file_type = (char *)malloc(sizeof(char) * 3);
 			if (!(temp->file_type)) {
+				file_context_node_destroy(temp);
+				free(temp);
+				free(line_buf);
 				fprintf(stderr, "Error: failure allocating memory.\n");
+				fc_free_file_context_node_list(head);
 				return 1;
 			}
 
 			if( i + 2 >= line_len ) {
 				file_context_node_destroy(temp);
 				free(temp);
-
 				continue;
 			}
 
@@ -448,7 +496,6 @@ int main(int argc, char *argv[])
 			}
 
 			if (i == line_len) {
-
 				file_context_node_destroy(temp);
 				free(temp);
 				continue;
@@ -467,24 +514,23 @@ int main(int argc, char *argv[])
 		if (!temp->context) {
 			file_context_node_destroy(temp);
 			free(temp);
+			free(line_buf);
 			fprintf(stderr, "Error: failure allocating memory.\n");
+			fc_free_file_context_node_list(head);
 			return 1;
 		}
 
 		/* Set all the data about the regular
-			*  expression. */
+		 * expression. */
 		fc_fill_data(temp);
 
 		/* Link this line of code at the end of
-			*  the linked list. */
+		 * the linked list. */
 		current->next = temp;
 		current = current->next;
 		lines++;
-
-
-		free(line_buf);
-		line_buf = NULL;
 	}
+	free(line_buf);
 	fclose(in_file);
 
 	/* Create the bucket linked list from the earlier linked list. */
@@ -492,6 +538,12 @@ int main(int argc, char *argv[])
 	bcurrent = master =
 	    (file_context_bucket_t *)
 	    malloc(sizeof(file_context_bucket_t));
+	if (!bcurrent) {
+		printf
+		    ("Error: failure allocating memory.\n"); 
+		fc_free_file_context_node_list(head);
+		return -1;
+	}
 	bcurrent->next = NULL;
 	bcurrent->data = NULL;
 
@@ -512,7 +564,9 @@ int main(int argc, char *argv[])
 			if (!(bcurrent->next)) {
 				printf
 				    ("Error: failure allocating memory.\n");
-				exit(-1);
+				free(head);
+				fc_free_file_context_bucket_list(master);
+				return -1;
 			}
 
 			/* Make sure the new bucket thinks it's the end of the
@@ -521,16 +575,19 @@ int main(int argc, char *argv[])
 
 			bcurrent = bcurrent->next;
 		}
-
 	}
 
 	/* Sort the bucket list. */
 	fc_merge_sort(master);
 
+	free(head);
+
 	/* Open the output file. */
 	if (output_name) {
 		if (!(out_file = fopen(output_name, "w"))) {
 			printf("Error: failure opening output file for write.\n");
+			fc_free_file_context_node_list(master->data);
+			free(master);
 			return -1;
 		}
 	} else {
@@ -539,6 +596,7 @@ int main(int argc, char *argv[])
 
 	/* Output the sorted file_context linked list to the output file. */
 	current = master->data;
+
 	while (current) {
 		/* Output the path. */
 		fprintf(out_file, "%s\t\t", current->path);
@@ -551,14 +609,10 @@ int main(int argc, char *argv[])
 		/* Output the context. */
 		fprintf(out_file, "%s\n", current->context);
 
-		/* Remove the node. */
-		temp = current;
 		current = current->next;
-
-		file_context_node_destroy(temp);
-		free(temp);
-
 	}
+
+	fc_free_file_context_node_list(master->data);
 	free(master);
 
 	if (output_name) {