Fedora Account System
Red Hat Associate
Red Hat Customer
Patch attached that fixes some memory leaks (not all) of logrotate, reported by Valgrind. Some things may look ugly, feel free to reject the patch. :-) Known memory leaks not fixed by this patch are tabooExts in config.c:249, some data of optCon in logrotate.c when poptGetNextOpt is called and the data of argv[argNum] when called by glob in config.c:966.
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