On Fri, Jul 13, 2018 at 2:04 PM Jeff King <peff@xxxxxxxx> wrote: > > On Fri, Jul 13, 2018 at 01:58:05PM -0700, Stefan Beller wrote: > > > > I'm sad that this strcpy() wasn't caught in review. IMHO we should avoid > > > that function altogether, even when we _think_ it can't trigger an > > > overflow. That's easier to reason about (and makes auditing easier). > > > > Can we somehow automatically find "bad code" either in pathces > > or in new code (such as pu), e.g. as a coccicheck for these functions? > > I'd be happy to declare strcpy() totally banned (and it more or less > is). I found this with a simple "git grep", though it seems like a > trivial application of coccinelle to find it. The question is what to > convert it into. into some "meta BUG("your review process failed")"? :-) > xsnprintf() is often a good choice, but not always > (e.g., if the destination isn't an array, we'd have to get the size from > somewhere else). > > I wouldn't be surprised if there's a way to ask coccinelle to convert > the easy cases and barf with an error on the hard cases or something. I > don't know the tool very well. I was just suggesting that tool as it is run on pu by some automation, hence it would not fall through the cracks. I mean how often to you happen to run git grep looking for strcpy on our code base and do we want to rely on that in the long run? Stefan