Bug 169888

Created attachment 119620 [details]
Fixes for memory leaks

Created attachment 119955 [details]
fixes segfault introduced in previous patch + minor cleanup

Fixed segfault, but valgrind still reports lost blocks. Didn't have time to
debug and patch those. Sorry.

More info on the segfault:

Parts of the following block realloc'ed at config.c:

   *logsPtr = realloc(*logsPtr, sizeof(**logsPtr) * *numLogsPtr);

were passed free() in the patch at logrotate.c (free_loginfo):

    for (i = 0; i < *numLogsPtr; i++)
        free(logsPtr[i]);

Personally, I would consider including some C++ code to avoid similar problems
in the future in the logrotate package. Just an opinion though.

Created attachment 120029 [details]
Patch to free memory allocated for tabooExts.

Thanks trzareq, you may see I am not a experienced programmer. I attached
another patch, this time to free memory allocated for tabooExts. There is still
one problem with tabooExts, because I have not found a proper solution to free
the memory allocated at end of the code below (of config.c), where is a comment
about it.
		if (!isolateValue(configFile, lineNum, "tabooext", &start,
				  &endtag)) {
		    oldchar = *endtag, *endtag = '\0';

		    if (*start == '+') {
			start++;
			while (isspace(*start) && *start) start++;
		    } else {
			free(tabooExts);
			tabooCount = 0;
			tabooExts = malloc(1);
		    }

		    while (*start) {
			chptr = start;
			while (!isspace(*chptr) && *chptr != ',' && *chptr)
			    chptr++;

			tabooExts = realloc(tabooExts, sizeof(*tabooExts) *
						(tabooCount + 1));
			/* this is a memory leak if the list gets reset */
			tabooExts[tabooCount] = malloc(chptr - start + 1);
			strncpy(tabooExts[tabooCount], start, chptr - start);
			tabooExts[tabooCount][chptr - start] = '\0';
			tabooCount++;

Apart from this problem, Valgrind only reports data leaked by poptGetNextOpt
(logrotate.c), then I think this maybe a problem with the popt library itself.

Created attachment 120034 [details]
Same patch as before, with a little change to make readConfigPath static.

Created attachment 120053 [details]
Maybe the final version of the patch to free tabooExts.

New version of the patch to fix leaks of tabooExts. I changed the way tabooExts
is allocated the first time, using malloc for each line of the array instead of
memcpy all pointers of defTabooExts. So I just free the array line-by-line,
when or not a new line is added by the code showed in the post above. To do
this I have renamed free_namelist to a more generic name (free_2d_array), for
use also with tabooExts, not only namelist. Valgrind now only reports leaks of
poptGetNextOpt (logrotate.c:1306), but there are calls to poptFreeContext for
every possible case, so I still think there is a problem with popt library.
Please, someone review this patch and, after applied it, close this bug.

I checked this patch. It seems to be right. Applied in logrotate-3.7.2-8