Re: [PATCH] cli: use designated initializers for opt desc

2017-10-01 Thread Jani Nikula 
On Sun, 01 Oct 2017, Tomi Ollila  wrote:
> On Sun, Oct 01 2017, Jani Nikula wrote:
>
>> On Sun, 01 Oct 2017, Tomi Ollila  wrote:
>>>
>>> Good stuff. It just doesn't longer compile on older compilers (so some
>>> note on commit message should be added):
>>
>> Does this on top fix it? I used the unnamed struct just for clarity, and
>> it doesn't serve a functional purpose.
>
> Yes it does!
>
> (output is noisy, but so is it with gcc 4.8.5 which I just tested to work)

What's so noisy about it...? :o

v2 as part of a bigger series at id:cover.1506890421.git.j...@nikula.org.

BR,
Jani.
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH] cli: use designated initializers for opt desc

2017-10-01 Thread Tomi Ollila 
On Sun, Oct 01 2017, Jani Nikula wrote:

> On Sun, 01 Oct 2017, Tomi Ollila  wrote:
>>
>> Good stuff. It just doesn't longer compile on older compilers (so some
>> note on commit message should be added):
>
> Does this on top fix it? I used the unnamed struct just for clarity, and
> it doesn't serve a functional purpose.

Yes it does!

(output is noisy, but so is it with gcc 4.8.5 which I just tested to work)

Thanks,

Tomi

>
> BR,
> Jani.
>
> diff --git a/command-line-arguments.h b/command-line-arguments.h
> index 97134ad535ee..799b7fef3f65 100644
> --- a/command-line-arguments.h
> +++ b/command-line-arguments.h
> @@ -16,15 +16,13 @@ typedef struct notmuch_keyword {
>  /* Describe one option. */
>  typedef struct notmuch_opt_desc {
>  /* One and only one of these must be set. */
> -struct {
> - const struct notmuch_opt_desc *opt_inherit;
> - notmuch_bool_t *opt_bool;
> - int *opt_int;
> - int *opt_keyword;
> - int *opt_flags;
> - const char **opt_string;
> - const char **opt_position;
> -};
> +const struct notmuch_opt_desc *opt_inherit;
> +notmuch_bool_t *opt_bool;
> +int *opt_int;
> +int *opt_keyword;
> +int *opt_flags;
> +const char **opt_string;
> +const char **opt_position;
>  
>  /* Must be set except for opt_inherit and opt_position. */
>  const char *name;
> --
>
>>
>> CC  -g -O2 notmuch.o
>> notmuch.c:53: error: unknown field ‘opt_bool’ specified in initializer
>> notmuch.c:53: warning: missing braces around initializer
>> notmuch.c:53: warning: (near initialization for
>> ‘notmuch_shared_options[0].’)
>> notmuch.c:53: warning: initialization from incompatible pointer type
>> notmuch.c:53: warning: missing initializer
>> notmuch.c:53: warning: (near initialization for
>> ‘notmuch_shared_options[0]..opt_bool’)
>> notmuch.c:54: error: unknown field ‘opt_bool’ specified in initializer
>> notmuch.c:54: warning: initialization from incompatible pointer type
>> notmuch.c:54: warning: missing initializer
>> notmuch.c:54: warning: (near initialization for
>> ‘notmuch_shared_options[1]..opt_bool’)
>> notmuch.c:55: error: unknown field ‘opt_string’ specified in initializer
>> notmuch.c:55: warning: initialization from incompatible pointer type
>> notmuch.c:55: warning: missing initializer
>> notmuch.c:55: warning: (near initialization for
>> ‘notmuch_shared_options[2]..opt_bool’)
>> notmuch.c:56: warning: missing initializer
>> notmuch.c:56: warning: (near initialization for
>> ‘notmuch_shared_options[3].’)
>> notmuch.c: In function ‘notmuch_minimal_options’:
>> notmuch.c:85: error: unknown field ‘opt_inherit’ specified in initializer
>> notmuch.c:85: warning: missing braces around initializer
>> notmuch.c:85: warning: (near initialization for ‘options[0].’)
>> notmuch.c:85: warning: missing initializer
>> notmuch.c:85: warning: (near initialization for
>> ‘options[0]..opt_bool’)
>> notmuch.c:86: warning: missing initializer
>> notmuch.c:86: warning: (near initialization for ‘options[1].’)
>> notmuch.c: In function ‘main’:
>> notmuch.c:414: error: unknown field ‘opt_string’ specified in initializer
>> notmuch.c:414: warning: missing braces around initializer
>> notmuch.c:414: warning: (near initialization for ‘options[0].’)
>> notmuch.c:414: warning: initialization from incompatible pointer type
>> notmuch.c:414: warning: missing initializer
>> notmuch.c:414: warning: (near initialization for
>> ‘options[0]..opt_bool’)
>> notmuch.c:415: error: unknown field ‘opt_inherit’ specified in initializer
>> notmuch.c:415: warning: missing initializer
>> notmuch.c:415: warning: (near initialization for
>> ‘options[1]..opt_bool’)
>> notmuch.c:416: warning: missing initializer
>> notmuch.c:416: warning: (near initialization for ‘options[2].’)
>> make: *** [notmuch.o] Error 1
>>
>> gcc version 4.4.6 20110731 (Red Hat 4.4.6-3) (GCC) 
>>
>> This was on Scientific Linux 6.2 -- will test on CentOS 7 (which IIRC has
>> gcc 4.8) container soon...
>>
>>
>> Tomi
>> ___
>> notmuch mailing list
>> notmuch@notmuchmail.org
>> https://notmuchmail.org/mailman/listinfo/notmuch
> ___
> notmuch mailing list
> notmuch@notmuchmail.org
> https://notmuchmail.org/mailman/listinfo/notmuch
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH] cli: use designated initializers for opt desc

2017-10-01 Thread Jani Nikula 
On Sun, 01 Oct 2017, Tomi Ollila  wrote:
> On Sun, Oct 01 2017, Jani Nikula wrote:
>
>> Several changes at once, just to not have to change the same lines
>> several times over:
>>
>> - Use designated initializers to initialize opt desc arrays.
>>
>> - Only initialize the needed fields.
>>
>> - Remove arg_id (short options) as unused.
>>
>> - Replace opt_type and output_var with several type safe output
>>   variables, where the output variable being non-NULL determines the
>>   type. Introduce checks to ensure only one is set. The downside is
>>   some waste of const space per argument; this could be saved by
>>   retaining opt_type and using a union, but that's still pretty
>>   verbose.
>>
>> - Fix some variables due to the type safety. Mostly a good thing, but
>>   leads to some enums being changed to ints. This is pedantically
>>   correct, but somewhat annoying. We could also cast, but that defeats
>>   the purpose a bit.
>>
>> - Terminate the opt desc arrays using {}.
>>
>> The output variable type safety and the ability to add new fields for
>> just some output types or arguments are the big wins. For example, if
>> we wanted to add a variable to set when the argument is present, we
>> could do so for just the arguments that need it.
>>
>> Beauty is in the eye of the beholder, but I think this looks nice when
>> defining the arguments, and reduces some of the verbosity we have
>> there.
>
> Good stuff. It just doesn't longer compile on older compilers (so some
> note on commit message should be added):

Does this on top fix it? I used the unnamed struct just for clarity, and
it doesn't serve a functional purpose.

BR,
Jani.

diff --git a/command-line-arguments.h b/command-line-arguments.h
index 97134ad535ee..799b7fef3f65 100644
--- a/command-line-arguments.h
+++ b/command-line-arguments.h
@@ -16,15 +16,13 @@ typedef struct notmuch_keyword {
 /* Describe one option. */
 typedef struct notmuch_opt_desc {
 /* One and only one of these must be set. */
-struct {
-   const struct notmuch_opt_desc *opt_inherit;
-   notmuch_bool_t *opt_bool;
-   int *opt_int;
-   int *opt_keyword;
-   int *opt_flags;
-   const char **opt_string;
-   const char **opt_position;
-};
+const struct notmuch_opt_desc *opt_inherit;
+notmuch_bool_t *opt_bool;
+int *opt_int;
+int *opt_keyword;
+int *opt_flags;
+const char **opt_string;
+const char **opt_position;
 
 /* Must be set except for opt_inherit and opt_position. */
 const char *name;
--

>
> CC  -g -O2 notmuch.o
> notmuch.c:53: error: unknown field ‘opt_bool’ specified in initializer
> notmuch.c:53: warning: missing braces around initializer
> notmuch.c:53: warning: (near initialization for
> ‘notmuch_shared_options[0].’)
> notmuch.c:53: warning: initialization from incompatible pointer type
> notmuch.c:53: warning: missing initializer
> notmuch.c:53: warning: (near initialization for
> ‘notmuch_shared_options[0]..opt_bool’)
> notmuch.c:54: error: unknown field ‘opt_bool’ specified in initializer
> notmuch.c:54: warning: initialization from incompatible pointer type
> notmuch.c:54: warning: missing initializer
> notmuch.c:54: warning: (near initialization for
> ‘notmuch_shared_options[1]..opt_bool’)
> notmuch.c:55: error: unknown field ‘opt_string’ specified in initializer
> notmuch.c:55: warning: initialization from incompatible pointer type
> notmuch.c:55: warning: missing initializer
> notmuch.c:55: warning: (near initialization for
> ‘notmuch_shared_options[2]..opt_bool’)
> notmuch.c:56: warning: missing initializer
> notmuch.c:56: warning: (near initialization for
> ‘notmuch_shared_options[3].’)
> notmuch.c: In function ‘notmuch_minimal_options’:
> notmuch.c:85: error: unknown field ‘opt_inherit’ specified in initializer
> notmuch.c:85: warning: missing braces around initializer
> notmuch.c:85: warning: (near initialization for ‘options[0].’)
> notmuch.c:85: warning: missing initializer
> notmuch.c:85: warning: (near initialization for
> ‘options[0]..opt_bool’)
> notmuch.c:86: warning: missing initializer
> notmuch.c:86: warning: (near initialization for ‘options[1].’)
> notmuch.c: In function ‘main’:
> notmuch.c:414: error: unknown field ‘opt_string’ specified in initializer
> notmuch.c:414: warning: missing braces around initializer
> notmuch.c:414: warning: (near initialization for ‘options[0].’)
> notmuch.c:414: warning: initialization from incompatible pointer type
> notmuch.c:414: warning: missing initializer
> notmuch.c:414: warning: (near initialization for
> ‘options[0]..opt_bool’)
> notmuch.c:415: error: unknown field ‘opt_inherit’ specified in initializer
> notmuch.c:415: warning: missing initializer
> notmuch.c:415: warning: (near initialization for
> ‘options[1]..opt_bool’)
> notmuch.c:416: warning: missing initializer
> notmuch.c:416: warning: (near initialization for ‘options[2].’)
> make: *** [notmuch.o] Error 1
>
> gcc version 4.4.6 20110731 (Red Hat 4.4.6-3) (GCC) 
>
> This wa

Re: [PATCH] cli: use designated initializers for opt desc

2017-10-01 Thread Tomi Ollila 
On Sun, Oct 01 2017, Jani Nikula wrote:

> Several changes at once, just to not have to change the same lines
> several times over:
>
> - Use designated initializers to initialize opt desc arrays.
>
> - Only initialize the needed fields.
>
> - Remove arg_id (short options) as unused.
>
> - Replace opt_type and output_var with several type safe output
>   variables, where the output variable being non-NULL determines the
>   type. Introduce checks to ensure only one is set. The downside is
>   some waste of const space per argument; this could be saved by
>   retaining opt_type and using a union, but that's still pretty
>   verbose.
>
> - Fix some variables due to the type safety. Mostly a good thing, but
>   leads to some enums being changed to ints. This is pedantically
>   correct, but somewhat annoying. We could also cast, but that defeats
>   the purpose a bit.
>
> - Terminate the opt desc arrays using {}.
>
> The output variable type safety and the ability to add new fields for
> just some output types or arguments are the big wins. For example, if
> we wanted to add a variable to set when the argument is present, we
> could do so for just the arguments that need it.
>
> Beauty is in the eye of the beholder, but I think this looks nice when
> defining the arguments, and reduces some of the verbosity we have
> there.

Good stuff. It just doesn't longer compile on older compilers (so some
note on commit message should be added):

CC  -g -O2 notmuch.o
notmuch.c:53: error: unknown field ‘opt_bool’ specified in initializer
notmuch.c:53: warning: missing braces around initializer
notmuch.c:53: warning: (near initialization for
‘notmuch_shared_options[0].’)
notmuch.c:53: warning: initialization from incompatible pointer type
notmuch.c:53: warning: missing initializer
notmuch.c:53: warning: (near initialization for
‘notmuch_shared_options[0]..opt_bool’)
notmuch.c:54: error: unknown field ‘opt_bool’ specified in initializer
notmuch.c:54: warning: initialization from incompatible pointer type
notmuch.c:54: warning: missing initializer
notmuch.c:54: warning: (near initialization for
‘notmuch_shared_options[1]..opt_bool’)
notmuch.c:55: error: unknown field ‘opt_string’ specified in initializer
notmuch.c:55: warning: initialization from incompatible pointer type
notmuch.c:55: warning: missing initializer
notmuch.c:55: warning: (near initialization for
‘notmuch_shared_options[2]..opt_bool’)
notmuch.c:56: warning: missing initializer
notmuch.c:56: warning: (near initialization for
‘notmuch_shared_options[3].’)
notmuch.c: In function ‘notmuch_minimal_options’:
notmuch.c:85: error: unknown field ‘opt_inherit’ specified in initializer
notmuch.c:85: warning: missing braces around initializer
notmuch.c:85: warning: (near initialization for ‘options[0].’)
notmuch.c:85: warning: missing initializer
notmuch.c:85: warning: (near initialization for
‘options[0]..opt_bool’)
notmuch.c:86: warning: missing initializer
notmuch.c:86: warning: (near initialization for ‘options[1].’)
notmuch.c: In function ‘main’:
notmuch.c:414: error: unknown field ‘opt_string’ specified in initializer
notmuch.c:414: warning: missing braces around initializer
notmuch.c:414: warning: (near initialization for ‘options[0].’)
notmuch.c:414: warning: initialization from incompatible pointer type
notmuch.c:414: warning: missing initializer
notmuch.c:414: warning: (near initialization for
‘options[0]..opt_bool’)
notmuch.c:415: error: unknown field ‘opt_inherit’ specified in initializer
notmuch.c:415: warning: missing initializer
notmuch.c:415: warning: (near initialization for
‘options[1]..opt_bool’)
notmuch.c:416: warning: missing initializer
notmuch.c:416: warning: (near initialization for ‘options[2].’)
make: *** [notmuch.o] Error 1

gcc version 4.4.6 20110731 (Red Hat 4.4.6-3) (GCC) 

This was on Scientific Linux 6.2 -- will test on CentOS 7 (which IIRC has
gcc 4.8) container soon...


Tomi
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH] cli: use designated initializers for opt desc

2017-10-01 Thread David Bremner 
Jani Nikula  writes:

> Several changes at once, just to not have to change the same lines
> several times over:

the general approach here looks fine. I didn't see any blockers, but
I'll wait a few days before merging.

This probably causes some rebasing pain for other people, but I think
it's one of those "rip the band-aid off quickly" scenarios.

d
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

[PATCH] cli: use designated initializers for opt desc

2017-09-30 Thread Jani Nikula 
Several changes at once, just to not have to change the same lines
several times over:

- Use designated initializers to initialize opt desc arrays.

- Only initialize the needed fields.

- Remove arg_id (short options) as unused.

- Replace opt_type and output_var with several type safe output
  variables, where the output variable being non-NULL determines the
  type. Introduce checks to ensure only one is set. The downside is
  some waste of const space per argument; this could be saved by
  retaining opt_type and using a union, but that's still pretty
  verbose.

- Fix some variables due to the type safety. Mostly a good thing, but
  leads to some enums being changed to ints. This is pedantically
  correct, but somewhat annoying. We could also cast, but that defeats
  the purpose a bit.

- Terminate the opt desc arrays using {}.

The output variable type safety and the ability to add new fields for
just some output types or arguments are the big wins. For example, if
we wanted to add a variable to set when the argument is present, we
could do so for just the arguments that need it.

Beauty is in the eye of the beholder, but I think this looks nice when
defining the arguments, and reduces some of the verbosity we have
there.
---
 command-line-arguments.c | 87 ++--
 command-line-arguments.h | 40 +-
 notmuch-client.h |  2 +-
 notmuch-compact.c|  8 ++---
 notmuch-count.c  | 16 -
 notmuch-dump.c   | 14 
 notmuch-insert.c | 14 
 notmuch-new.c| 12 +++
 notmuch-reindex.c|  4 +--
 notmuch-reply.c  | 12 +++
 notmuch-restore.c| 14 
 notmuch-search.c | 46 -
 notmuch-show.c   | 22 ++--
 notmuch-tag.c| 12 +++
 notmuch.c| 22 ++--
 test/arg-test.c  | 21 ++--
 test/hex-xcode.c | 10 +++---
 test/random-corpus.c | 20 +--
 18 files changed, 188 insertions(+), 188 deletions(-)

diff --git a/command-line-arguments.c b/command-line-arguments.c
index dc517b06ff60..f1a5b2324337 100644
--- a/command-line-arguments.c
+++ b/command-line-arguments.c
@@ -22,12 +22,10 @@ _process_keyword_arg (const notmuch_opt_desc_t *arg_desc, 
char next, const char
 
 while (keywords->name) {
if (strcmp (arg_str, keywords->name) == 0) {
-   if (arg_desc->output_var) {
-   if (arg_desc->opt_type == NOTMUCH_OPT_KEYWORD_FLAGS)
-   *((int *)arg_desc->output_var) |= keywords->value;
-   else
-   *((int *)arg_desc->output_var) = keywords->value;
-   }
+   if (arg_desc->opt_flags)
+   *arg_desc->opt_flags |= keywords->value;
+   else
+   *arg_desc->opt_keyword = keywords->value;
return TRUE;
}
keywords++;
@@ -43,15 +41,15 @@ static notmuch_bool_t
 _process_boolean_arg (const notmuch_opt_desc_t *arg_desc, char next, const 
char *arg_str) {
 
 if (next == '\0') {
-   *((notmuch_bool_t *)arg_desc->output_var) = TRUE;
+   *arg_desc->opt_bool = TRUE;
return TRUE;
 }
 if (strcmp (arg_str, "false") == 0) {
-   *((notmuch_bool_t *)arg_desc->output_var) = FALSE;
+   *arg_desc->opt_bool = FALSE;
return TRUE;
 }
 if (strcmp (arg_str, "true") == 0) {
-   *((notmuch_bool_t *)arg_desc->output_var) = TRUE;
+   *arg_desc->opt_bool = TRUE;
return TRUE;
 }
 fprintf (stderr, "Unknown argument \"%s\" for (boolean) option \"%s\".\n", 
arg_str, arg_desc->name);
@@ -67,7 +65,7 @@ _process_int_arg (const notmuch_opt_desc_t *arg_desc, char 
next, const char *arg
return FALSE;
 }
 
-*((int *)arg_desc->output_var) = strtol (arg_str, &endptr, 10);
+*arg_desc->opt_int = strtol (arg_str, &endptr, 10);
 if (*endptr == '\0')
return TRUE;
 
@@ -87,10 +85,35 @@ _process_string_arg (const notmuch_opt_desc_t *arg_desc, 
char next, const char *
fprintf (stderr, "String argument for option \"%s\" must be 
non-empty.\n", arg_desc->name);
return FALSE;
 }
-*((const char **)arg_desc->output_var) = arg_str;
+*arg_desc->opt_string = arg_str;
 return TRUE;
 }
 
+/* Return number of non-NULL opt_* fields in opt_desc. */
+static int _opt_set_count (const notmuch_opt_desc_t *opt_desc)
+{
+return
+   !!opt_desc->opt_inherit +
+   !!opt_desc->opt_bool +
+   !!opt_desc->opt_int +
+   !!opt_desc->opt_keyword +
+   !!opt_desc->opt_flags +
+   !!opt_desc->opt_string +
+   !!opt_desc->opt_position;
+}
+
+/* Return TRUE if opt_desc is valid. */
+static notmuch_bool_t _opt_valid (const notmuch_opt_desc_t *opt_desc)
+{
+int n = _opt_set_count (opt_desc);
+
+if (n > 1)
+   INTERNAL_ERROR ("more than one non-NULL opt_* field for argument 
\"%s\"",
+   opt_desc->nam