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

> Jeff King <p...@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

Reply via email to