Re: [PATCH 7/9] builtin-checkout-index.c: use parse_options()

From: René Scharfe <rene.scharfe@lsrfire.ath.cx>
Date: Jul 25, 2008, 09:01 UTC
Message-ID: <48899657.5090209@lsrfire.ath.cx>
In-Reply-To: <bd6139dc0807241335i3ab5280aq6a46325428ccc70f@mail.gmail.com>

Sverre Rabbelier schrieb:

Show 15 quoted lines

> On Thu, Jul 24, 2008 at 10:08 PM, Michele Ballabio
> <barra_cuda@katamail.com> wrote:
>> On Thursday 24 July 2008, Johannes Schindelin wrote:
>>> On Wed, 23 Jul 2008, Michele Ballabio wrote:
>>>
>>>> +           { OPTION_CALLBACK, 'f', "force", &state, NULL,
>>>> +             "force overwrite of existing files",
>>>> +             PARSE_OPT_NOARG, parse_state_force_cb, 0 },
>>> I wonder if this could not be written as
>>>
>>>               OPT_BOOLEAN('f', "force", &state.force,
>>>                       "force overwrite of existing files"),
>> I did it that way because 'force' is a bitfield.
> 
> I thought there is an OPT_BIT?

OPT_BIT is for flags and bitmasks, not for bitfields.

Since you can't get the address of a bitfield member, a function that wants to change its value needs to know its name. Switching to bitmasks would make the option parsing code look cleaner, but you'd have to change all those bitfield accesses to explicit bitmask operations, e.g.:

	if (state.force)
		state.force = 0;

vs.

	if (state.flags & CHECKOUT_FORCE)
		state.flags &= ~CHECKOUT_FORCE;

In the case of struct checkout, though, we could simply make the bitfield members full ints, because there are only a few instances of this structure in memory at any given time. Wasting a few bytes of RAM in order to gain much simpler code is OK in this case, I think. OPT_BOOLEAN looks a lot nicer than a callback.

René