Jeff King <peff@xxxxxxxx> writes:
> FWIW, I agree with you here. These patches are not making anything worse
> (and may even make them better, since we'd probably need to swap out the
> BLOCKSIZE constant for a run-time "blocksize" variable in fewer places).
It's just that leaving the interface uneven is an easy way to
introduce an unnecessary bug, e.g.
-type function(args) {
+type function(args, size_t blocksize) {
decls;
- helper_one(BLOCKSIZE, other, args);
+ helper_one(blocksize, other, args);
helper_two(its, args);
- helper_three(BLOCKSIZE, even, more, args);
+ helper_three(blocksize, even, more, args);
}
when this caller is away from the implementation of helper_two()
that hardcodes the assumption that this callchain only uses
BLOCKSIZE and in an implicit way.
And that can easily be avoided by defensively making helper_two() to
take BLOCKSIZE as an argument as everybody else in the caller does.
I do not actually care too deeply, though. Hopefully whoever adds
"-b" would be careful enough to follow all callchain, and at least
look at all the callees that are file-scope static, and the one I
have trouble with _is_ a file-scope static.
Or maybe nobody does "-b", in which case this ticking time bomb will
not trigger, so we'd be OK.
