Re: [RFC PATCH 1/5] ref-filter: add objectsize:disk option

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 13, 2018, 01:52 UTC
Message-ID: <xmqqwophyc9s.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <20181112131247.GL3956@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 21 quoted lines

>> You mean something like
>> 
>> 			v->s = xstrfmt("%"PRIdMAX, (intmax_t)oi->disk_size);
>
> I think elsewhere we simply use PRIuMAX for printing large sizes via
> off_t; we know this value isn't going to be negative.
>
> I'm not opposed to PRIdMAX, which _is_ more accurate, but...
>
>> P.S.: I wondered whether we have precedent for PRIdMAX, as we used to use
>> only PRIuMAX, but yes: JeffH's json-writer uses PRIdMAX.
>
> That's pretty recent. I won't be surprised if we have to do some
> preprocessor trickery to handle that at some point. We have a PRIuMAX
> fallback already. That comes from c4001d92be (Use off_t when we really
> mean a file offset., 2007-03-06), but it's not clear to me if that was
> motivated by a real platform or an over-abundance of caution.
>
> I'm OK with just using PRIdMAX as appropriate for now. It will serve as
> a weather-balloon, and we can #define our way out of it later if need
> be.

I am OK if we avoid PRIdMAX and use PRIuMAX instead with a cast to the corresponding size in this codepath, as long as we properly handle negative oi.disk_size field, which may be telling some "unusual" condition to us.