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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Sverre Rabbelier schrieb:
> On Thu, Jul 24, 2008 at 10:08 PM, Michele Ballabio
> <barra_cuda@xxxxxxxxxxxx> 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é
--
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

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux