On Tue, Oct 10, 2023 at 07:45:41PM -0400, Taylor Blau wrote:
> On Mon, Oct 09, 2023 at 04:58:23PM -0400, Jeff King wrote:
> > There are no callers of the "safe" version yet, but we'll add some in
> > subsequent patches.
>
> Makes sense.
>
> > +int pair_chunk_unsafe(struct chunkfile *cf,
> > + uint32_t chunk_id,
> > + const unsigned char **p)
> > {
> > - return read_chunk(cf, chunk_id, pair_chunk_fn, p);
> > + size_t dummy;
> > + return pair_chunk(cf, chunk_id, p, &dummy);
>
> I have always disliked functions that require you to pass a non-NULL
> pointer to some value that you may or may not want to have that function
> fill out. So I was going to suggest something along the lines of
> "pair_chunk() should tolerate a NULL fourth argument instead of filling
> out the size unconditionally".
>
> But that's (a) the whole point of the series ;-), and (b) unnecessary,
> since this function is going to go away entirely by the end of the
> series, too.
Yeah, for the record, I think a dummy variable like this is usually a
code smell. And it truly is a "problem" here, because we are
intentionally doing the unsafe and stupid thing. :)
-Peff
