Jeff King [off-list ref] 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.
But that is something we do not have to worry about until somebody
tries to shrink the structure by making these flags into bitfields.
Thanks for an updated patch.
On Wed, Aug 22, 2018 at 02:50:05PM -0700, Junio C Hamano wrote:
Jeff King [off-list ref] writes:
quoted
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