On Thu, Oct 10, 2024 at 09:00:32AM +0200, Patrice Dumas wrote:
> Hello,
>
> There are warnings with -Wmissing-field-initializers for info/variables.c
> VARIABLE_ALIST info_variables where the where_set field is never
> explicitely set. This code is from Gavin, and is by design, as there
> is this comment:
> /* Note that the 'where_set' field of each element in the array is
> not given and defaults to 0. */
>
> I would have put 0 there myself if I had written that code, but omitting
> fields seems to me to be also ok if it is documented as it is here.
> However, the warning is also interesting to set as in other cases the
> missing fields may be an overlook, and having false positives is a pain.
>
> Should I add the 0 and remove the comments or leave it as is?
They do default to 0. Maybe it's an obscure detail of the C language.
Here it is in the C99 standard:
If there are fewer initializers in a brace-enclosed list than there
are elements or members of an aggregate, or fewer characters in a
string literal used to initialize an array of known size than there
are elements in the array, the remainder of the aggregate shall be
initialized implicitly the same as objects that have static storage
duration.
(section 6.7.8, paragraph 21).
I can see the argument that it may look like a mistake and it would be
better if someone reading the code didn't have to make sense of how
this worked.
I don't like having to have an extra entry of 0 in all of the initialisers
as this is redundant and could be a source of error.
It could be possible to remove where_set from 'info_variables' and keep
the values of 'info_variables' unchanged while the program runs. This
information could be in a separate array that is the same length as
the 'info_variables' array.
Another idea is to use preprocessor macros to generate parts of the
initialiser, which macros would add the ", 0" automatically. There
could be e.g. ON_OFF_VAR so instead of writing
{ "visible-bell",
N_("When \"On\", flash the screen instead of ringing the bell"),
&terminal_use_visible_bell_p, (char **)on_off_choices, 0 },
you would write something like
ON_OFF_VAR("visible_bell",
N_("When \"On\", flash the screen instead of ringing the bell"),
terminal_use_visible_bell_p),
There would also be macros for other types of variable, like
CHOICES_VAR("mouse",
N_("Method to use to track mouse events"),
mouse_protocol, mouse_choices),
instead of
{ "mouse",
N_("Method to use to track mouse events"),
&mouse_protocol, (char **)mouse_choices, 0 },
I'd be happy to make this change myself if nobody has a better idea.
