Hi Tim,
I wrote:
> Fixed like this. Let's see what remaining issues Coverity reports in glob.c
> (next Monday).
Now, coverity does not report any resource leak for lib/glob.c any more. But
IMO your analysis about the memory leak is still mostly correct. Except around
line 1070, where your patch would add a double-free, I think you're right.
So, I've applied this change. I'm not introducing a new macro RETURNVAL
because that does not seem to fit with the glibc coding style.
2017-07-10 Tim Rühsen <tim.rueh...@gmx.de>
Bruno Haible <br...@clisp.org>
glob: Fix more memory leaks.
* lib/glob.c (glob): Use 'goto out' in order to free dirname before
returning.
Reported by Tim Rühsen.
diff --git a/lib/glob.c b/lib/glob.c
index a38cf22..3b3194a 100644
--- a/lib/glob.c
+++ b/lib/glob.c
@@ -1039,9 +1039,12 @@ glob (const char *pattern, int flags, int (*errfunc)
(const char *, int),
free (malloc_pwtmpbuf);
if (flags & GLOB_TILDE_CHECK)
- /* We have to regard it as an error if we cannot find the
- home directory. */
- return GLOB_NOMATCH;
+ {
+ /* We have to regard it as an error if we cannot find the
+ home directory. */
+ retval = GLOB_NOMATCH;
+ goto out;
+ }
}
}
}
@@ -1068,12 +1071,11 @@ glob (const char *pattern, int flags, int (*errfunc)
(const char *, int),
if (newcount > SIZE_MAX / sizeof (char *) - 2)
{
nospace:
- if (__glibc_unlikely (malloc_dirname))
- free (dirname);
free (pglob->gl_pathv);
pglob->gl_pathv = NULL;
pglob->gl_pathc = 0;
- return GLOB_NOSPACE;
+ retval = GLOB_NOSPACE;
+ goto out;
}
new_gl_pathv = realloc (pglob->gl_pathv,
@@ -1113,7 +1115,8 @@ glob (const char *pattern, int flags, int (*errfunc)
(const char *, int),
}
/* Not found. */
- return GLOB_NOMATCH;
+ retval = GLOB_NOMATCH;
+ goto out;
}
meta = __glob_pattern_type (dirname, !(flags & GLOB_NOESCAPE));
@@ -1159,7 +1162,10 @@ glob (const char *pattern, int flags, int (*errfunc)
(const char *, int),
if (status != 0)
{
if ((flags & GLOB_NOCHECK) == 0 || status != GLOB_NOMATCH)
- return status;
+ {
+ retval = status;
+ goto out;
+ }
goto no_matches;
}
@@ -1178,7 +1184,8 @@ glob (const char *pattern, int flags, int (*errfunc)
(const char *, int),
if (interrupt_state)
{
globfree (&dirs);
- return GLOB_ABORTED;
+ retval = GLOB_ABORTED;
+ goto out;
}
}
#endif /* SHELL. */
@@ -1197,7 +1204,8 @@ glob (const char *pattern, int flags, int (*errfunc)
(const char *, int),
globfree (&dirs);
globfree (pglob);
pglob->gl_pathc = 0;
- return status;
+ retval = status;
+ goto out;
}
/* Stick the directory on the front of each name. */
@@ -1208,7 +1216,8 @@ glob (const char *pattern, int flags, int (*errfunc)
(const char *, int),
globfree (&dirs);
globfree (pglob);
pglob->gl_pathc = 0;
- return GLOB_NOSPACE;
+ retval = GLOB_NOSPACE;
+ goto out;
}
}
@@ -1230,7 +1239,8 @@ glob (const char *pattern, int flags, int (*errfunc)
(const char *, int),
{
nospace2:
globfree (&dirs);
- return GLOB_NOSPACE;
+ retval = GLOB_NOSPACE;
+ goto out;
}
new_gl_pathv = realloc (pglob->gl_pathv,
@@ -1245,7 +1255,8 @@ glob (const char *pattern, int flags, int (*errfunc)
(const char *, int),
globfree (&dirs);
globfree (pglob);
pglob->gl_pathc = 0;
- return GLOB_NOSPACE;
+ retval = GLOB_NOSPACE;
+ goto out;
}
++pglob->gl_pathc;
@@ -1257,7 +1268,8 @@ glob (const char *pattern, int flags, int (*errfunc)
(const char *, int),
else
{
globfree (&dirs);
- return GLOB_NOMATCH;
+ retval = GLOB_NOMATCH;
+ goto out;
}
}
@@ -1303,7 +1315,8 @@ glob (const char *pattern, int flags, int (*errfunc)
(const char *, int),
flags = orig_flags;
goto no_matches;
}
- return status;
+ retval = status;
+ goto out;
}
if (dirlen > 0)
@@ -1315,7 +1328,8 @@ glob (const char *pattern, int flags, int (*errfunc)
(const char *, int),
{
globfree (pglob);
pglob->gl_pathc = 0;
- return GLOB_NOSPACE;
+ retval = GLOB_NOSPACE;
+ goto out;
}
}
}
@@ -1340,7 +1354,8 @@ glob (const char *pattern, int flags, int (*errfunc)
(const char *, int),
{
globfree (pglob);
pglob->gl_pathc = 0;
- return GLOB_NOSPACE;
+ retval = GLOB_NOSPACE;
+ goto out;
}
strcpy (&new[len - 2], "/");
pglob->gl_pathv[i] = new;
