[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openbsd-tech
Subject:    [patch] memory leaks in opencvs:modules.c
From:       Igor Zinovik <zinovik () cs ! karelia ! ru>
Date:       2008-02-25 17:11:13
Message-ID: 20080225171112.GH8568 () kappa ! cs ! prv
[Download RAW message or body]

	Hello, tech@ readers.

cvs_module_lookup() losses memory when it founds a module that is
requested.  Module checkout structure `mc' is allocated when function
starts.  Then it tries to find module with name `name' and if it
(function) finds it (module) memory is allocated to variable that
already hold some memory.  Seems that second xmalloc() call is
redundant.

--- modules.c.orig	2008-02-25 19:43:42.000000000 +0300
+++ modules.c	2008-02-25 19:56:43.000000000 +0300
@@ -215,7 +215,6 @@ cvs_module_lookup(char *name)
 
         TAILQ_FOREACH(mi, &modules, m_list) {
                 if (!strcmp(name, mi->mi_name)) {
-                        mc = xmalloc(sizeof(*mc));
                         mc->mc_modules = mi->mi_modules;
                         mc->mc_ignores = mi->mi_ignores;
                         mc->mc_canfree = 0;

Function `module_parse_line' does not free `bline' and `prog'.  `bline'
holds copy of `line', but when things go bad and we leave function it
does not freed.  `prog' gets memory when we are parsing `-o' or `-i'
flags.  My fix is blunt.  I think it is better to create some kind of
label `relase_memory' and add goto statements that will point to it, but
i personally do not like `goto'.
			 	
--- modules.c.orig	2008-02-25 19:43:42.000000000 +0300
+++ modules.c	2008-02-25 19:50:54.000000000 +0300
@@ -94,6 +94,9 @@ modules_parse_line(char *line, int linen
                 switch (val[1]) {
                 case 'a':
                         if (flags & MODULE_TARGETDIR) {
+                                if (prog != NULL)
+                                        xfree(prog);
+                                xfree(bline);
                                 cvs_log(LP_NOTICE, "cannot use -a with -d");
                                 return;
                         }
@@ -101,6 +104,9 @@ modules_parse_line(char *line, int linen
                         break;
                 case 'd':
                         if (flags & MODULE_ALIAS) {
+                                if (prog != NULL)
+                                        xfree(prog);
+                                xfree(bline);
                                 cvs_log(LP_NOTICE, "cannot use -d with -a");
                                 return;
                         }
@@ -111,6 +117,8 @@ modules_parse_line(char *line, int linen
                         break;
                 case 'o':
                         if (flags != 0 || prog != NULL) {
+                                xfree(prog);
+                                xfree(bline);
                                 cvs_log(LP_NOTICE,
                                     "-o cannot be used with other flags");
                                 return;
@@ -129,6 +137,8 @@ modules_parse_line(char *line, int linen
                         break;
                 case 'i':
                         if (flags != 0 || prog != NULL) {
+                                xfree(prog);
+                                xfree(bline);
                                 cvs_log(LP_NOTICE,
                                     "-i cannot be used with other flags");
                                 return;
@@ -202,6 +212,10 @@ modules_parse_line(char *line, int linen
         return;
 
 bad:
+        if (prog != NULL)
+                xfree(prog);
+        xfree(bline);
+
         cvs_log(LP_NOTICE, "malformed line in CVSROOT/modules: %d", lineno);
 }

[prev in list] [next in list] [prev in thread] [next in thread]