On Fri, Jul 08, 2022 at 11:47:58PM +0200, Ævar Arnfjörð Bjarmason wrote: > But we also have a few other uses of malloc() in the codebase. I wonder > if the right thing here isn't to just use malloc(), but to have > git-compat-util.h override malloc() (similar to how we we override > e.g. exit() now...), which would also catch those. I suspect that introduces new complexities, as some calls really may want an actual no-frills malloc. I'm thinking stuff in compat/, for example, where extra actions taken by xmalloc() could cause weird looping or races. This was probably a lot more likely when we closed packs via xmalloc (e.g., via git_mmap()'s malloc() call) but I think the general principle still holds. E.g., gitsetenv() calling getenv() is a potential questionable area. It does look like the call in submodule--helper.c is just wrong, though, and should be changed. You could probably detect these with the preprocessor, but again, you run into complexities with the cases that _should_ be vanilla malloc. Given how little of a problem this has been historically, I'm mostly content to notice these in review and occasionally grep for fixes. I suppose a coccinelle rule could help, because it makes it easy to suppress false positives (like all of compat/), which the preprocessor doesn't. -Peff
