Re: [PATCH] rev-list: make empty --stdin not an error

From: Jeff King <peff@peff.net>
Date: Aug 22, 2018, 21:55 UTC
Message-ID: <20180822215501.GA27737@sigill.intra.peff.net>
In-Reply-To: <xmqqr2iqkr5e.fsf@gitster-ct.c.googlers.com>

On Wed, Aug 22, 2018 at 02:50:05PM -0700, Junio C Hamano wrote:

Show 21 quoted lines

> Jeff King <peff@peff.net> writes:
> 
> > Yes. I was thinking it had more purpose than this, but it really is just
> > a flag to check "did we do this already?". Which is one of the main
> > purposes I claimed for the new flag in my commit message. :)
> 
> OK.  
> 
> The reason I was on the fence was primarily because read_from_stdin
> field in the structure observable from outside can be a boolean
> (that is, "unsigned :1"), but internally this may want to count up
> to two.
> 
> Or with "unsigned read_from_stdin:1", would this 
> 
> 	if (revs->read_from_stdin++)
> 		die("twice???");
> 
> still be usable?  As the value of post-increment would be 1 even
> when the resulting field would have wrapped-around already, it
> should be OK, but it just felt strange to me.

I agree it would work in practice, though I also agree it is funny and should be avoided.

> But that is something we do not have to worry about until somebody
> tries to shrink the structure by making these flags into bitfields.

Also agreed. I'd probably resolve it then by writing:

  if (revs->read_from_stdin)
	die("twice");
  revs->read_from_stdin = 1;

I guess we could even do that now. Or add a test to make sure "--stdin --stdin" barfs. But I am perfectly happy to punt until somebody actually wants to use a bitfield.

-Peff