On Wed, Jun 29 2022, Phillip Wood via GitGitGadget wrote: > This patch series introduces macros for allocating and growing arrays in > xdiff. The macros are similar to ALLOC_ARRAY()/ALLOC_GROW() from the rest of > the code base but return an error on failure to allow libgit2 to handle > memory allocation failures gracefully rather than dying. The macros > introduce overflow checks but these checks are currently redundant as we > limit the maximum file size passed to xdiff and these checks alone are > insufficient to safely remove the size limit. The aim of this series is to > make the xdiff code more readable, there should be no change in behavior (as > such I'm open to the argument that these are just churn and should be > dropped). I think it's a good direction, but why not make such new macros non-XDL_* specific, add them to git-compat-util.h, and then define our existing macros that call xmalloc() now in terms of these new macros? I realize that it'll take a bit more careful hacking in wrapper.c and git-compat-util.h, but it would allow us to eventually make some other low-level APIs of ours use such an API. E.g. we have some hand-rolled replacements for "struct strbuf" in at least a couple of places (e.g. vreportf() in usage.c). If you pull on that thread you'll see that it's for no reason other than strbuf.c calls ALLOC_GROW(), which we'll die() in, and we don't want to die on malloc failure in e.g. BUG().
