[PATCH] fts: avoid a memory leak edge case

[Pádraig Brady]

Hi,
I may be wrong but I suspect there is a corner case where fts_close()
will not free the FTSENT structures correctly if called immediately
after fts_open().
After fts_open(), the current entry is a dummy entry created as
if ((sp->fts_cur = fts_alloc(sp, "", 0)) == NULL)
goto mem3;
sp->fts_cur->fts_link = root;
sp->fts_cur->fts_info = FTS_INIT;
It would normally be freed during the first invocation of fts_read().
if (sp->fts_cur) {
for (p = sp->fts_cur; p->fts_level >= FTS_ROOTLEVEL;) {
freep = p;
p = p->fts_link != NULL ? p->fts_link : p->fts_parent;
free(freep);
}
free(p);
}
However, fts_alloc() does not clear or set fts_level, nor does it zero
the entire FTSENT structure.
So as far as I can figure, it is possible for the fts_level of the
dummy entry to be negative after fts_open() causing fts_close() not to
free the actual root level entries.

Yes valgrind indicates that fts_level is uninitialized if you
fts_close() right after fts_open().
The attached should fix it up.

thanks!
Pádraig

[Kamil Dudka]

@@ -122,9 +139,10 @@ main (void)
perror_exit (base, 6);
while ((e = fts_read (ftsp)))
needles_seen += strcmp (e->fts_name, "needle") == 0;
- fflush (stdout);
if (errno)
perror_exit ("fts_read", 7);
+ if (fts_close (ftsp) != 0)
+ perror_exit (base, 8);
/* Report an error if we did not find the needles. */
if (needles_seen != needles)

Why are you removing fflush (stdout) from the test without any explanation?

Kamil

[Bruno Haible]

Post by Kamil Dudka
Why are you removing fflush (stdout) from the test without any explanation?

Yes, fflush(stdout) statements are extremely important if you want to
understand/debug test failures on native Windows.

Bruno

[Pádraig Brady]

Post by Bruno Haible

Post by Kamil Dudka
Why are you removing fflush (stdout) from the test without any explanation?

Yes, fflush(stdout) statements are extremely important if you want to
understand/debug test failures on native Windows.

Oops, I wasn't aware of windows considerations.

I didn't actually see where there was any output to stdout
in this test, and I thought fflush() was therefore redundant?
More problematically could impinge on the previous fts_read() errno?

Should save the errno from fts_read?
Should I perror() any errno from fflush?

cheers,
Pádraig