From: Matthew DeVore Date: Mon, 03 Dec 2018 22:01:12 GMT Subject: Re: [PATCH] revisions.c: put promisor option in specialized struct Message-ID: <0809e859-b3a6-79ef-031c-9ff3326da31d@comcast.net> In-Reply-To: <20181203212431.GB8700@sigill.intra.peff.net> On 12/03/2018 01:24 PM, Jeff King wrote: >> @@ -297,7 +296,8 @@ struct setup_revision_opt { >> const char *def; >> void (*tweak)(struct rev_info *, struct setup_revision_opt *); >> const char *submodule; /* TODO: drop this and use rev_info->repo */ >> - int assume_dashdash; >> + int assume_dashdash : 1; >> + int allow_exclude_promisor_objects : 1; >> unsigned revarg_opt; >> }; > > I don't know that we need to penny-pinch bytes in this struct, but in > general it shouldn't hurt either awy. However, a signed bit-field with 1 > bit is funny. I'm not even sure what the standard has to say, but in > twos-complement that would store "-1" and "0" (gcc -Wpedantic also > complains about overflow in assigning "1" to it). Interesting. I hadn't suspected this. But I confirmed it with this: #include struct x { int y : 1; int z : 1; }; int main() { struct x x; x.y = 1; x.z = 1; printf("%d %d\n", (int) x.y, (int) x.z); return 0; } -- Output -- -1 -1 > > So this probably ought to be "unsigned". Earlier in this file we define bit fields this way: /* Traversal flags */ unsigned int dense:1, prune:1, ... using \t to align the field names, so I'll mimic that style.