"Dr. David Alan Gilbert" <dgilb...@redhat.com> writes:
> * Markus Armbruster (arm...@redhat.com) wrote:
>> "Dr. David Alan Gilbert" <dgilb...@redhat.com> writes:
>>
>> > * Markus Armbruster (arm...@redhat.com) wrote:
>> >> The has_FOO for pointer-valued FOO are redundant, except for arrays.
>> >> They are also a nuisance to work with. Recent commit "qapi: Start to
>> >> elide redundant has_FOO in generated C" provided the means to elide
>> >> them step by step. This is the step for qapi/misc.json.
>> >>
>> >> Said commit explains the transformation in more detail. The invariant
>> >> violations mentioned there do not occur here.
>> >>
>> >> Cc: Dr. David Alan Gilbert <dgilb...@redhat.com>
>> >> Signed-off-by: Markus Armbruster <arm...@redhat.com>
[...]
>> >> diff --git a/util/qemu-config.c b/util/qemu-config.c
>> >> index 5325f6bf80..95f61fc883 100644
>> >> --- a/util/qemu-config.c
>> >> +++ b/util/qemu-config.c
>> >> @@ -80,14 +80,8 @@ static CommandLineParameterInfoList
>> >> *query_option_descs(const QemuOptDesc *desc)
>> >> break;
>> >> }
>> >>
>> >> - if (desc[i].help) {
>> >> - info->has_help = true;
>> >> - info->help = g_strdup(desc[i].help);
>> >> - }
>> >> - if (desc[i].def_value_str) {
>> >> - info->has_q_default = true;
>> >> - info->q_default = g_strdup(desc[i].def_value_str);
>> >> - }
>> >> + info->help = g_strdup(desc[i].help);
>> >> + info->q_default = g_strdup(desc[i].def_value_str);
>> >>
>> >> QAPI_LIST_PREPEND(param_list, info);
>> >> }
>> >> @@ -245,8 +239,7 @@ static QemuOptsList machine_opts = {
>> >> }
>> >> };
>> >>
>> >> -CommandLineOptionInfoList *qmp_query_command_line_options(bool
>> >> has_option,
>> >> - const char
>> >> *option,
>> >> +CommandLineOptionInfoList *qmp_query_command_line_options(const char
>> >> *option,
>> >> Error **errp)
>> >> {
>> >> CommandLineOptionInfoList *conf_list = NULL;
>> >> @@ -254,7 +247,7 @@ CommandLineOptionInfoList
>> >> *qmp_query_command_line_options(bool has_option,
>> >> int i;
>> >>
>> >> for (i = 0; vm_config_groups[i] != NULL; i++) {
>> >> - if (!has_option || !strcmp(option, vm_config_groups[i]->name)) {
>> >> + if (!option || !strcmp(option, vm_config_groups[i]->name)) {
>> >
>> > I think that can be g_strcmp0 if you can convince yourself ->name is
>> > non-null
>>
>> vm_config_groups[i] must not be null.
>>
>> However, replacing the whole condition by !g_strcmp0() would be wrong:
>>
>> option | null ->name neither
>> -----------------------------------+------------------------
>> !option || !strcmp(option, ->name) | true true false
>> g_strcmp0(option, ->name) | false true false
>
> Oops yes, sorry these are the other way around as you point out.
I fell into the exact same trap myself :)
[...]
