Re: [PATCH 10/10] reftable: address trivial -Wsign-compare warnings

[Patrick Steinhardt <ps@pks.im>]

On Thu, Jan 16, 2025 at 02:12:33PM -0800, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> 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