Re: [PATCH v2 2/4] archive-tar: mark RECORDSIZE/BLOCKSIZE as unsigned

From: Jeff King <peff@peff.net>
Date: May 8, 2019, 23:04 UTC
Message-ID: <20190508230420.GC19990@sigill.intra.peff.net>
In-Reply-To: <nycvar.QRO.7.76.6.1905081334260.44@tvgsbejvaqbjf.bet>

On Wed, May 08, 2019 at 01:45:25PM +0200, Johannes Schindelin wrote:

Show 18 quoted lines

> Hi René,
> 
> On Thu, 2 May 2019, René Scharfe wrote:
> 
> > Am 27.04.19 um 01:27 schrieb Johannes Schindelin via GitGitGadget:
> > > From: Johannes Schindelin <johannes.schindelin@gmx.de>
> > >
> > > They really are unsigned, and we are using e.g. BLOCKSIZE as `size_t`
> > > parameter to pass to `write_or_die()`.
> >
> > True, but the compiler converts that value correctly to size_t without
> > complaint already, doesn't it?  What am I missing?
> 
> Are you talking about a specific compiler? It sure sounds as if you did.
> 
> I really do not want to fall into the "you can build Git with *any*
> compiler, as long as that compiler happens to be GCC, oh, and as long it
> is version X" trap.

I don't this this has anything to do with gcc. The point is that we already have this line:

  write_or_die(fd, buf, BLOCKSIZE);

which does not cast and nobody has complained, even though the signed constant is implicitly converted to a size_t. So adding another line like:

  gzwrite(gzip, block, BLOCKSIZE);

would in theory be treated the same (gzwrite takes an "unsigned").

The conversion from signed to unsigned is well defined in ANSI C, and I'd expect a compiler to either complain about neither or both (and the latter probably with warnings like -Wconversion cranked up).

But of course if you have data otherwise, we can revise that. Was the cast added out of caution, or to squelch a compiler warning?

-Peff