fc_sort: memory leakages

Avoid memory leakages in the fc_sort executable (now passes
all valgrind AND Clang static analyzer 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 7821eb6f37 as such
trick is no longer needed, given that all memory leakages
have now been fixed.

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

For reference, the original issue reported by the Cland
static analyzer is as follows:

support/fc_sort.c:494:6: warning: Potential leak of memory
pointed to by 'head'
            malloc(sizeof(file_context_bucket_t));

Signed-off-by: Guido Trentalancia <guido@trentalancia.com>
Acked-by: William Roberts <william.c.roberts@intel.com>
This commit is contained in:
Guido Trentalancia 2017-10-04 23:02:21 +02:00 committed by Chris PeBenito
parent 7821eb6f37
commit 5490639ac9
1 changed files with 69 additions and 39 deletions

View File

@ -46,6 +46,9 @@ typedef struct file_context_node {
void file_context_node_destroy(file_context_node_t *x) void file_context_node_destroy(file_context_node_t *x)
{ {
if (!x)
return;
free(x->path); free(x->path);
free(x->file_type); free(x->file_type);
free(x->context); free(x->context);
@ -135,8 +138,6 @@ file_context_node_t *fc_merge(file_context_node_t *a,
file_context_node_t *temp; file_context_node_t *temp;
file_context_node_t *jumpto; file_context_node_t *jumpto;
/* If a is a empty list, and b is not, /* If a is a empty list, and b is not,
* set a as b and proceed to the end. */ * set a as b and proceed to the end. */
if (!a && b) if (!a && b)
@ -164,7 +165,6 @@ file_context_node_t *fc_merge(file_context_node_t *a,
fc_compare(a_current->next, fc_compare(a_current->next,
b_current) != -1) { b_current) != -1) {
temp = a_current->next; temp = a_current->next;
a_current->next = b_current; a_current->next = b_current;
b_current = b_current->next; b_current = b_current->next;
@ -177,7 +177,6 @@ file_context_node_t *fc_merge(file_context_node_t *a,
a_current = jumpto; a_current = jumpto;
} }
/* if there is anything left in b to be inserted, /* if there is anything left in b to be inserted,
put it on the end */ put it on the end */
if (b_current) { if (b_current) {
@ -209,11 +208,12 @@ file_context_node_t *fc_merge(file_context_node_t *a,
*/ */
void fc_merge_sort(file_context_bucket_t *master) void fc_merge_sort(file_context_bucket_t *master)
{ {
file_context_bucket_t *current; file_context_bucket_t *current;
file_context_bucket_t *temp; file_context_bucket_t *temp;
if (!master)
return;
/* Loop until master is the only bucket left /* Loop until master is the only bucket left
* so that this will stop when master contains * so that this will stop when master contains
* the sorted list. */ * the sorted list. */
@ -222,28 +222,20 @@ void fc_merge_sort(file_context_bucket_t *master)
/* This loop merges buckets two-by-two. */ /* This loop merges buckets two-by-two. */
while (current) { while (current) {
if (current->next) { if (current->next) {
current->data = current->data =
fc_merge(current->data, fc_merge(current->data,
current->next->data); current->next->data);
temp = current->next; temp = current->next;
current->next = current->next->next; current->next = current->next->next;
free(temp); free(temp);
} }
current = current->next; current = current->next;
} }
} }
} }
@ -307,6 +299,25 @@ void fc_fill_data(file_context_node_t *fc_node)
} }
} }
/* 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;
}
}
/* main /* main
* This program takes in two arguments, the input filename and the * This program takes in two arguments, the input filename and the
* output filename. The input file should be syntactically correct. * output filename. The input file should be syntactically correct.
@ -328,7 +339,6 @@ int main(int argc, char *argv[])
FILE *in_file, *out_file; FILE *in_file, *out_file;
/* Check for the correct number of command line arguments. */ /* Check for the correct number of command line arguments. */
if (argc < 2 || argc > 3) { if (argc < 2 || argc > 3) {
fprintf(stderr, "Usage: %s <infile> [<outfile>]\n",argv[0]); fprintf(stderr, "Usage: %s <infile> [<outfile>]\n",argv[0]);
@ -348,24 +358,33 @@ int main(int argc, char *argv[])
/* Initialize the head of the linked list. */ /* Initialize the head of the linked list. */
head = current = (file_context_node_t*)malloc(sizeof(file_context_node_t)); 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->next = NULL;
head->path = NULL;
head->file_type = NULL;
head->context = NULL;
/* Parse the file into a file_context linked list. */ /* Parse the file into a file_context linked list. */
line_buf = NULL; line_buf = NULL;
while ( getline(&line_buf, &buf_len, in_file) != -1 ){ while ( getline(&line_buf, &buf_len, in_file) != -1 ){
line_len = strlen(line_buf); line_len = strlen(line_buf);
if( line_len == 0 || line_len == 1) if( line_len == 0 || line_len == 1)
continue; continue;
/* Get rid of whitespace from the front of the line. */ /* Get rid of whitespace from the front of the line. */
for (i = 0; i < line_len; i++) { for (i = 0; i < line_len; i++) {
if (!isspace(line_buf[i])) if (!isspace(line_buf[i]))
break; break;
} }
if (i >= line_len) if (i >= line_len)
continue; continue;
/* Check if the line isn't empty and isn't a comment */ /* Check if the line isn't empty and isn't a comment */
if (line_buf[i] == '#') if (line_buf[i] == '#')
continue; continue;
@ -373,7 +392,9 @@ int main(int argc, char *argv[])
/* We have a valid line - allocate a new node. */ /* We have a valid line - allocate a new node. */
temp = (file_context_node_t *)malloc(sizeof(file_context_node_t)); temp = (file_context_node_t *)malloc(sizeof(file_context_node_t));
if (!temp) { if (!temp) {
free(line_buf);
fprintf(stderr, "Error: failure allocating memory.\n"); fprintf(stderr, "Error: failure allocating memory.\n");
fc_free_file_context_node_list(head);
return 1; return 1;
} }
temp->next = NULL; temp->next = NULL;
@ -382,19 +403,15 @@ int main(int argc, char *argv[])
/* Parse out the regular expression from the line. */ /* Parse out the regular expression from the line. */
start = i; start = i;
while (i < line_len && (!isspace(line_buf[i]))) while (i < line_len && (!isspace(line_buf[i])))
i++; i++;
finish = i; finish = i;
regex_len = finish - start; regex_len = finish - start;
if (regex_len == 0) { if (regex_len == 0) {
file_context_node_destroy(temp); file_context_node_destroy(temp);
free(temp); free(temp);
continue; continue;
} }
@ -402,13 +419,14 @@ int main(int argc, char *argv[])
if (!temp->path) { if (!temp->path) {
file_context_node_destroy(temp); file_context_node_destroy(temp);
free(temp); free(temp);
free(line_buf);
fprintf(stderr, "Error: failure allocating memory.\n"); fprintf(stderr, "Error: failure allocating memory.\n");
fc_free_file_context_node_list(head);
return 1; return 1;
} }
/* Get rid of whitespace after the regular expression. */ /* Get rid of whitespace after the regular expression. */
for (; i < line_len; i++) { for (; i < line_len; i++) {
if (!isspace(line_buf[i])) if (!isspace(line_buf[i]))
break; break;
} }
@ -420,18 +438,21 @@ int main(int argc, char *argv[])
} }
/* Parse out the type from the line (if it /* Parse out the type from the line (if it
* is there). */ * is there). */
if (line_buf[i] == '-') { if (line_buf[i] == '-') {
temp->file_type = (char *)malloc(sizeof(char) * 3); temp->file_type = (char *)malloc(sizeof(char) * 3);
if (!(temp->file_type)) { if (!(temp->file_type)) {
file_context_node_destroy(temp);
free(temp);
free(line_buf);
fprintf(stderr, "Error: failure allocating memory.\n"); fprintf(stderr, "Error: failure allocating memory.\n");
fc_free_file_context_node_list(head);
return 1; return 1;
} }
if( i + 2 >= line_len ) { if( i + 2 >= line_len ) {
file_context_node_destroy(temp); file_context_node_destroy(temp);
free(temp); free(temp);
continue; continue;
} }
@ -448,7 +469,6 @@ int main(int argc, char *argv[])
} }
if (i == line_len) { if (i == line_len) {
file_context_node_destroy(temp); file_context_node_destroy(temp);
free(temp); free(temp);
continue; continue;
@ -467,24 +487,23 @@ int main(int argc, char *argv[])
if (!temp->context) { if (!temp->context) {
file_context_node_destroy(temp); file_context_node_destroy(temp);
free(temp); free(temp);
free(line_buf);
fprintf(stderr, "Error: failure allocating memory.\n"); fprintf(stderr, "Error: failure allocating memory.\n");
fc_free_file_context_node_list(head);
return 1; return 1;
} }
/* Set all the data about the regular /* Set all the data about the regular
* expression. */ * expression. */
fc_fill_data(temp); fc_fill_data(temp);
/* Link this line of code at the end of /* Link this line of code at the end of
* the linked list. */ * the linked list. */
current->next = temp; current->next = temp;
current = current->next; current = current->next;
lines++; lines++;
free(line_buf);
line_buf = NULL;
} }
free(line_buf);
fclose(in_file); fclose(in_file);
/* Create the bucket linked list from the earlier linked list. */ /* Create the bucket linked list from the earlier linked list. */
@ -492,6 +511,12 @@ int main(int argc, char *argv[])
bcurrent = master = bcurrent = master =
(file_context_bucket_t *) (file_context_bucket_t *)
malloc(sizeof(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->next = NULL;
bcurrent->data = NULL; bcurrent->data = NULL;
@ -512,25 +537,33 @@ int main(int argc, char *argv[])
if (!(bcurrent->next)) { if (!(bcurrent->next)) {
printf printf
("Error: failure allocating memory.\n"); ("Error: failure allocating memory.\n");
exit(-1); free(head);
fc_free_file_context_node_list(current);
fc_merge_sort(master);
fc_free_file_context_node_list(master->data);
free(master);
return -1;
} }
/* Make sure the new bucket thinks it's the end of the /* Make sure the new bucket thinks it's the end of the
* list. */ * list. */
bcurrent->next->next = NULL; bcurrent->next->next = NULL;
bcurrent = bcurrent->next; bcurrent = bcurrent->next;
} }
} }
/* Sort the bucket list. */ /* Sort the bucket list. */
fc_merge_sort(master); fc_merge_sort(master);
free(head);
/* Open the output file. */ /* Open the output file. */
if (output_name) { if (output_name) {
if (!(out_file = fopen(output_name, "w"))) { if (!(out_file = fopen(output_name, "w"))) {
printf("Error: failure opening output file for write.\n"); printf("Error: failure opening output file for write.\n");
fc_free_file_context_node_list(master->data);
free(master);
return -1; return -1;
} }
} else { } else {
@ -539,6 +572,7 @@ int main(int argc, char *argv[])
/* Output the sorted file_context linked list to the output file. */ /* Output the sorted file_context linked list to the output file. */
current = master->data; current = master->data;
while (current) { while (current) {
/* Output the path. */ /* Output the path. */
fprintf(out_file, "%s\t\t", current->path); fprintf(out_file, "%s\t\t", current->path);
@ -551,14 +585,10 @@ int main(int argc, char *argv[])
/* Output the context. */ /* Output the context. */
fprintf(out_file, "%s\n", current->context); fprintf(out_file, "%s\n", current->context);
/* Remove the node. */
temp = current;
current = current->next; current = current->next;
file_context_node_destroy(temp);
free(temp);
} }
fc_free_file_context_node_list(master->data);
free(master); free(master);
if (output_name) { if (output_name) {