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

From: Jeff King <peff@peff.net>
Date: Nov 12, 2018, 13:12 UTC
Message-ID: <20181112131247.GL3956@sigill.intra.peff.net>
In-Reply-To: <nycvar.QRO.7.76.6.1811121300520.39@tvgsbejvaqbjf.bet>

On Mon, Nov 12, 2018 at 01:03:25PM +0100, Johannes Schindelin wrote:

Show 15 quoted lines

> > oi.disk_size is off_t; do we know "long long" 
> > 
> >    (1) is available widely enough (I think it is from c99)?
> > 
> >    (2) is sufficiently wide so that we can safely cast off_t to?
> > 
> >    (3) will stay to be sufficiently wide as machines get larger
> >        together with standard types like off_t in the future?
> > 
> > I'd rather use intmax_t (as off_t is a signed integral type) so that
> > we do not have to worry about these questions in the first place.
> 
> 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.

-Peff