On Thu, Apr 11, 2019 at 12:14:52PM -0700, Taylor Blau wrote:
> > This series cleans up most of the bare calls found by:
> >
> > git grep -E '[^a-z_](m|c|re)alloc\(' '*.c' :^compat :^contrib :^wrapper.c
>
> This (admittedly pretty awesome) 'git grep' invocation reminds me of a
> series I was pretty sure you wrote to ban functions like 'strcpy' and
> other obviously bad things.
>
> Some quick searching turned up [1], which landed as f225611d1c
> (automatically ban strcpy(), 2018-07-26). Do we want something similar
> here? Of course, the locations below would have to be exempt, but it
> seems worthwhile (and would save a review cycle in the case that someone
> added a 'malloc' in a patch sent here).
I don't think we can ban malloc, since we have to use it ourselves. :)
With some contortions, we probably could unban it specifically in
wrapper.c (though note there are a few other calls I've left which would
need to be handled somehow).
Another option would be coccinelle patches to convert malloc() to
xmalloc(), etc (with an exception for the wrappers). I'm not entirely
comfortable with automatic conversion here because there's often some
follow-up adjustments (i.e., we can stop handling allocation errors and
maybe delete some code). I think coccinelle can identify callers and
barf, though.
I'm not sure whether coccinelle saves review cycles (since that implies
people actually run it, though maybe that is better now that it's part
of Travis?). It seems to me that it's usually more helpful for people
periodically doing follow-up auditing.
So I dunno. If this was a common mistake I'd be more concerned with
saving review cycles, but all of the cases I found were actually just
leftovers from the very early days of Git.
-Peff
