Nguyen Thai Ngoc Duy <pclouds@xxxxxxxxx> writes: >> Now reference_name stores the result of xstrdup(), it does not have reason >> to be of type "const char *". It is preferable to lose the cast here, I >> think. The same comment applies to the remainder of the patch. > > But resolve_ref() returns "const char *", we need to type cast at > least once, either at resolve_ref() assignment or at free(), until we > change resolve_ref(). In any case, I do not think it matters either way, so I queued this patch to 'pu' unmodified. This patch uses xstrdup() on return value of resolve_ref() only at hand-picked places; while the choice of the places the patch decided not to call free() looked reasonable from a quick review, both of us may be blind and it may introduce huge leaks in a repeatedly called function. A more extensive patch that would turn resolve_ref() to return an allocated piece of memory share the same risk of adding new leaks at the callsites, and this late in the cycle, neither will be in 1.7.8 anyway. Given that if we build the more extensive patch on top of this one after 1.7.8, it will need to undo xstrdup() in this patch, add free()s that become necessary, in addition to having to add free()s that this patch might have potentially forgot, I have a feeling that we should just drop this [2/2] and do a more thorough fix after 1.7.8 release is done immediately on top of [1/2]. Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html