On 04/03/16 23:50, Junio C Hamano wrote: > Ramsay Jones <ram...@ramsayjones.plus.com> writes: > >> The xdl_prepare_env() function may initialise an xdlclassifier_t >> data structure via xdl_init_classifier(), which allocates memory >> to several fields, for example 'rchash', 'rcrecs' and 'ncha'. >> If this function later exits due to the failure of xdl_optimize_ctxs(), >> then this xdlclassifier_t structure, and the memory allocated to it, >> is not cleaned up. >> >> In order to fix the memory leak, insert a call to xdl_free_classifier() >> before returning. >> >> Signed-off-by: Ramsay Jones <ram...@ramsayjones.plus.com> >> --- >> xdiff/xprepare.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/xdiff/xprepare.c b/xdiff/xprepare.c >> index 5ffcf99..13b55ab 100644 >> --- a/xdiff/xprepare.c >> +++ b/xdiff/xprepare.c >> @@ -301,6 +301,7 @@ int xdl_prepare_env(mmfile_t *mf1, mmfile_t *mf2, >> xpparam_t const *xpp, >> >> xdl_free_ctx(&xe->xdf2); >> xdl_free_ctx(&xe->xdf1); >> + xdl_free_classifier(&cf); >> return -1; >> } > > This looks obviously correct from the pattern of prepare's and > free's in the code that this part follows. This potential leak has > been that way since 3443546f (Use a *real* built-in diff generator, > 2006-03-24), i.e. the very beginning. > > I find it somewhat strange that the call to xdl_free_classifier() at > the end of this function is made conditional to XDF_HISTOGRAM_DIFF, > though. I can half-buy the argument "that is because we do not call > init-classifier for XDF_HISTOGRAM_DIFF", but in the error path we > call free-classifier unconditionally, so the code clearly knows that > it is safe to call free-classifier on a classifier that is cleared > with the initial memset(&cf, 0, sizeof cf).
Indeed, this is actually why I noticed that XDF_DIFF_ALG() wasn't used. Rather than doing patch #1, I did consider making this call unconditional. I can't remember why I didn't. (Hmm, perhaps I just chickened out! ;-)) ATB, Ramsay Jones -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
