Re: [PATCH] Return a bitmask of found options from parse_opts()

[Nikola Pajkovsky]

On 10/08/2010 03:29 PM, Denys Vlasenko wrote:
>  This patch makes it easier to analyze "bool" type options
> -
>  now they are all just bits in a single return value,
>  no need to have an integer variable for each option.
>
>  The previous behavior is retained too: it is useful for
>  "cumulative" options like -vvv.
>  
cumulative options is done by  *(int*)opt[ii].value += 1, right?

>  The patch also removes all OPT_GROUPs.
>
>  Run tested. Please review.
>  
This is not wise.

+enum {
+    OPT_v = 1 << 0,
+    OPT_d = 1 << 1,
+    OPT_i = 1 << 2,
+    OPT_t = 1 << 3,
+    OPT_s = 1 << 4,
+};

+                if (ii < sizeof(retval)*8)
+                    retval |= (1 << ii);

this means that it's position related.

If I swap something in

static struct options abrt_action_generate_backtrace_options[] = {
      OPT__VERBOSE(&g_verbose),
-    OPT_GROUP(""),
-    OPT_STRING( 'd' , 0, &d_opt, "dir", "Crash dump
directory"),
+    OPT_STRING( 'd' , 0, &dump_dir_name, "dir", "Crash dump
directory"),
      OPT_STRING( 'i' , 0, &i_opt, "dir1[:dir2]...",
"Additional debuginfo directories"),
      OPT_INTEGER( 't' , 0, &exec_timeout_sec, "Kill gdb if it runs for
more than SECONDS"),
-    OPT_BOOL( 's' , 0, &s_opt, "Log to syslog even with -d"),
+    OPT_BOOL( 's' , 0, NULL, "Log to syslog even with -d"),
      OPT_END()
  };

it won't fit to enum.

And also I don't see any problem with that if arg is not preset then is set to 0(due
to static) or
is bump up by*(int*)opt[ii].value += 1

-- 
Nikola