On Wed, Aug 19, 2015 at 03:46:25PM -0400, Dave Borowitz wrote: > >> + unsigned dry_run = 0; > >> + unsigned send_mirror = 0; > >> + unsigned force_update = 0; > >> + unsigned quiet = 0; > >> + unsigned push_cert = 0; > >> + unsigned use_thin_pack = 0; > >> + unsigned atomic = 0; > >> + unsigned stateless_rpc = 0; > > > > First I thought: > > You could write to the args flags directly from the options. No > > need to have (most of) > > the variables around here and copy over the values. You'd need to > > use OPT_BIT instead > > for setting a specific bit though > > but then I realized we do not have a direct bit field in args, which > > would make it a bit unreadable. > > Right, and &args->push_cert etc. is invalid, and I didn't know if it > was ok to expand the args struct to be several words longer. But I'm > not a C programmer so I'm happy to take suggestions how to make this > more idiomatic. I think it would be fine to expand it. The reason to use bitfields is to save memory, and there is literally only one of these structs per program. I'm sure we can afford the extra dozen bytes. Making the struct members single-bits also communicates to readers that they are true booleans, but I think a comment in the declaration of send_pack_args could do the same. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html