On Thu, Jan 16, 2025 at 02:12:33PM -0800, Junio C Hamano wrote:
> Patrick Steinhardt <ps@xxxxxx> writes:
> > diff --git a/reftable/writer.c b/reftable/writer.c
> > index 4e6ca2e368..91d6629486 100644
> > --- a/reftable/writer.c
> > +++ b/reftable/writer.c
> > @@ -577,7 +577,7 @@ static int writer_finish_section(struct reftable_writer *w)
> >
> > struct common_prefix_arg {
> > struct reftable_buf *last;
> > - int max;
> > + size_t max;
> > };
>
> This is dubious. write.c:update_common() uses this to keep track of
> the maximum value returned by common_prefix_size(), which returns
> an int. writer.c:writer_dump_object_index() assigns the value
> comparable to this member to reftable_stats.object_id_len that is of
> type int.
>
> I may be more sympathetic if we were making common_prefix_size()
> return size_t instead of "int" and dealing with all the fallouts
> from such a change, but this smells more like somebody _else_ is
> using size_t on something that is not an allocation size, and such
> mixed use is cascading down to contaminate this member, which would
> be perfectly fine with platform native "int".
>
> Ah, OK, an earlier patch does change these other things to size_t,
> so this must change to size_t to be consistent. Shouldn't it be
> done in the same patch, then, though?
Good point indeed. I'll move the change around, thanks!
Patrick
