On Tue, 2006-06-27 at 18:08 -0700, Debora Velarde wrote: > Below is a patch for a resource leak found by Coverity. > > If the calloc call succeeds for 'new' //line 692 > and > new->class_name = strdup(token) returns NULL //line 699 > and > class_list = NULL //line 733 > > Then funtion returns without freeing new
This leaks free'ing new->class_name. The least intrusive fix for this
is:
diff -rup devallocator-orig/lib/parse.c devallocator/lib/parse.c
--- devallocator-orig/lib/parse.c 2006-07-11 01:18:02.000000000 -0400
+++ devallocator/lib/parse.c 2006-07-13 16:12:50.000000000 -0400
@@ -728,6 +728,8 @@ parse_class_conf(void)
error_out:
if (conf_file)
fclose(conf_file);
+ if (new->class_name)
+ free(new->class_name);
if (new)
free(new);
while (class_list != NULL) {
...but personally, I'd do:
diff -rup devallocator-orig/lib/parse.c devallocator/lib/parse.c
--- devallocator-orig/lib/parse.c 2006-07-11 01:18:02.000000000 -0400
+++ devallocator/lib/parse.c 2006-07-13 16:21:38.000000000 -0400
@@ -666,7 +666,7 @@ parse_class_conf(void)
conf_file = fopen(conf_file_path, "r");
if (conf_file == NULL)
- goto error_out;
+ goto error_file_out;
while (fgets(line, MAX_LINE_LENGTH, conf_file) != NULL) {
/* Make sure we have a complete line */
if (line[strlen(line)-1] != '\n') {
@@ -693,6 +693,8 @@ parse_class_conf(void)
"failure\n");
goto error_out;
}
+ new->next = class_list;
+ class_list = new;
new->class_name = strdup(token);
if (new->class_name == NULL) {
@@ -712,33 +714,23 @@ parse_class_conf(void)
"failure\n");
goto error_out;
}
-
- if (class_list == NULL) {
- class_list = new;
- } else {
- new->next = class_list;
- class_list = new;
- }
-
}
fclose(conf_file);
return class_list;
error_out:
- if (conf_file)
- fclose(conf_file);
- if (new)
- free(new);
+ fclose(conf_file);
while (class_list != NULL) {
new = class_list;
class_list = class_list->next;
- if (new->class_name)
- free(new->class_name);
- if (new->class_library)
- free(new->class_library);
+
+ free(new->class_name);
+ free(new->class_library);
free(new);
}
+
+error_file_out:
return NULL;
}
...does anyone have any objections to the later form?
--
James Antill <[EMAIL PROTECTED]>
signature.asc
Description: This is a digitally signed message part
-- redhat-lspp mailing list [email protected] https://www.redhat.com/mailman/listinfo/redhat-lspp
