flag setting issues with override

[Papp Gyozo (VBuster)]

Hi Lorenzo,

we've encountered a problem with flags with gengetopt 2.22.1/2. I know that you don't like flags very much but my app is using 14 flags. (for historical reasons, of course ;) Since it is a business product I couldn't change the option type so easily.

The problem arises when a flag option is set more than once in successive call of getoptions_internal(). In this case update_arg() flips back, eventually reset the flag_given while field_given is incremented twice:

if (field_given)
    (*field_given)++;

case ARG_FLAG:
    *((int *)field) = !*((int *)field);

I don't know and could not figure out by myself if it is the intended behaviour of gengetopt. Could you help me, please? If I change update_arg() it solve my problem:

-  if (field_given && *field_given && ! override)
+  if (field_given && *field_given && ( !override || arg_type == ARG_FLAG ))
    return 0;

but I don't know if it is the good thing (tm).

The detailed scenario is that config file is parsed first and then the command line. The latter has precedence over the former, that is, command line arguments overrides option values given in config file. In my understanding "override" should mean "confirm" in case of flags at least when different settings are originated from different sources (or different runs of gengetopt_internal()).

Here is the code flow:

    struct getoptions_params    ggo = {
                                    1, // override what is in cmdline struct
                                    1, // initialize struct w/ default values
                                    0, // check required
                                    0, // check ambiguity
                                    1  // print errors (getopt_long, too)
                                };

    getoptions_init ( options );
(...)
    ggo.override = 1, ggo.initialize = 0;
    if (   options->config_arg
        && getoptions_config_file ( options->config_arg, options, &ggo ) )
(...)
    if ( getoptions_ext ( argc, argv, options, &ggo ) != EXIT_SUCCESS )

Note that we should follow these approach. Otherwise, in reverse order, when override is set to 0, gengetopt raises error if a groupoption is set in both the command line and in the config file. (It might be another issue.)


--
Papp, Gyozo
VirusBuster Kft


_______________________________________________
Help-gengetopt mailing list
Help-gengetopt@...
http://lists.gnu.org/mailman/listinfo/help-gengetopt

[Papp Gyozo (VBuster)]

Hi there,

> The detailed scenario is that config file is parsed first and then the
> command line. The latter has precedence over the former, that is, command
> line arguments overrides option values given in config file. In my
> understanding "override" should mean "confirm" in case of flags at least
> when different settings are originated from different sources (or different
> runs of gengetopt_internal()).

In short term, I'm thinking of introducing a new parameter for the parser to check if flag is set more than once. This way we could preserve the original behaviour and support the new one as I suggested yesterday.

>     struct getoptions_params    ggo = {
>                                     1, // override what is in cmdline struct
>                                     1, // initialize struct w/ default
> values
>                                     0, // check required
>                                     0, // check ambiguity

check ambiguity would be fine but I avoid to overload with additional functionality.
The new one could be named as check flag or something like that to refer this problem.

In a long run, I would prefer to change flag option generation as to gcc's options.
I mean to generate to separate options for setting and reseting flags:
  -flag    # to enable (switch on) the `flag'
  -no-flag # to disable (switch off) the `flag'

I hope this way we could avoid several problems and misunderstandings around flag options. Unfortunately it breaks compatibility.


What do think?

Best regards,
Gyozo Papp


_______________________________________________
Help-gengetopt mailing list
Help-gengetopt@...
http://lists.gnu.org/mailman/listinfo/help-gengetopt

[Lorenzo Bettini]

Papp Gyozo (VBuster) wrote:
> Hi there,
>
>> The detailed scenario is that config file is parsed first and then the
>> command line. The latter has precedence over the former, that is, command
>> line arguments overrides option values given in config file. In my
>> understanding "override" should mean "confirm" in case of flags at least
>> when different settings are originated from different sources (or different
>> runs of gengetopt_internal()).
>

Hi there

I answer this email together with the first one.

Yes, as you said, I don't like flag options ;) that's something I had
already found in gengetopt when I started to maintain it.  Actually, I
don't know the real semantics myself, and I guess the same flag should
not be given twice on the command line (which includes also
configuration files).

But I understand your issue, but I cannot change the current behavior in
order not to break backward compatibility.  I'll go back to this in a
minute.

First, out of curiosity, why are you using flag options instead of
options without arguments?  Would you be able to simulate the same
behavior of flag options?  This is just my curiosity, in order to be
able to understand whether we're able to get rid off flag options someday :)

> In short term, I'm thinking of introducing a new parameter for the parser to check if flag is set more than once. This way we could preserve the original behaviour and support the new one as I suggested yesterday.
>
>>     struct getoptions_params    ggo = {
>>                                     1, // override what is in cmdline struct
>>                                     1, // initialize struct w/ default
>> values
>>                                     0, // check required
>>                                     0, // check ambiguity
>
> check ambiguity would be fine but I avoid to overload with additional functionality.
> The new one could be named as check flag or something like that to refer this problem.
>

This is the best solution, since adding a new parameter is backward
compatible, so how about override_flags?  If set to 0, this would
simulate your

if (field_given && *field_given && ( !override || arg_type == ARG_FLAG ))

i.e.,

if (field_given && *field_given && ( !override || (arg_type == ARG_FLAG
&& !override_flag)))

what do you think?

> In a long run, I would prefer to change flag option generation as to gcc's options.
> I mean to generate to separate options for setting and reseting flags:
>   -flag    # to enable (switch on) the `flag'
>   -no-flag # to disable (switch off) the `flag'
>

I was asked to add such functionality and I'd like to work on this, but
these would not be real flag options: it would be options without name
with an automatic generation of the '-no-' corresponding version

> I hope this way we could avoid several problems and misunderstandings around flag options. Unfortunately it breaks compatibility.
>

why would it break backward compatibility?  I mean the solution with
additional param should not break backward compatibility (if set by
default to 1).

cheers
        Lorenzo

--
Lorenzo Bettini, PhD in Computer Science, DI, Univ. Torino
ICQ# lbetto, 16080134     (GNU/Linux User # 158233)
HOME: http://www.lorenzobettini.it MUSIC: http://www.purplesucker.com
http://www.myspace.com/supertrouperabba
BLOGS: http://tronprog.blogspot.com  http://longlivemusic.blogspot.com
http://www.gnu.org/software/src-highlite
http://www.gnu.org/software/gengetopt
http://www.gnu.org/software/gengen http://doublecpp.sourceforge.net


_______________________________________________
Help-gengetopt mailing list
Help-gengetopt@...
http://lists.gnu.org/mailman/listinfo/help-gengetopt

[Papp Gyozo (VBuster)]

> First, out of curiosity, why are you using flag options instead of
> options without arguments?  Would you be able to simulate the same

To be frank, dunno. :) Now  I've realized from CVS that I used flags from the beginning (~ gengetop 2.11). I think now that flags seemed to the obvious choice for simple on/off options. I don't remember quite well but probably there might be some little glitches with simple options that time.

Just to be precise you are suggesting such changes like below?

option  "flag" f
        "real flag"
        flag off
        hidden

option  "flag" f
        "mimic flag"
        off
        hidden

> behavior of flag options?  This is just my curiosity, in order to be
> able to understand whether we're able to get rid off flag options someday :)

I think this change is really worth considering and should be targeted now for an _exact_ future release. (gengetopt 3.0?) I mean the next releases may emit warning messages on flag options, then successive releases refuse to accept them by default only if a specific option is set. (-compat-flag). Finally all the hassle should be left out.

> This is the best solution, since adding a new parameter is backward
> compatible, so how about override_flags?  If set to 0, this would
> simulate your
>
> if (field_given && *field_given && ( !override || arg_type == ARG_FLAG ))
>
> i.e.,
>
> if (field_given && *field_given && ( !override || (arg_type == ARG_FLAG
> && !override_flag)))
>
> what do you think?

It could work.
 

> > In a long run, I would prefer to change flag option generation as to gcc's
> options.
> > I mean to generate to separate options for setting and reseting flags:
> >   -flag    # to enable (switch on) the `flag'
> >   -no-flag # to disable (switch off) the `flag'
> >
>
> I was asked to add such functionality and I'd like to work on this, but
> these would not be real flag options: it would be options without name
> with an automatic generation of the '-no-' corresponding version.

It would be nice. Go for it :)
I proposed that the old flag semantics could be "recycled" this way.

> > I hope this way we could avoid several problems and misunderstandings
> around flag options. Unfortunately it breaks compatibility.
>
> why would it break backward compatibility?  I mean the solution with
> additional param should not break backward compatibility (if set by
> default to 1).

BC break will occur only if you drop the old flag behaviour and introduce this new -f/-no pairs under the same name This was what I suggested. Introducing a new param in the generator may handle BC.


_______________________________________________
Help-gengetopt mailing list
Help-gengetopt@...
http://lists.gnu.org/mailman/listinfo/help-gengetopt