Re: [PATCH] qemu-config: extract same logic in *_add_opts() to fill_config_groups()

["Wang, Lei" <lei4.wang@intel.com>]

Kindly ping for any comments.

BR,
Lei

On 9/2/2022 3:57 PM, Markus Armbruster wrote:
> Cc: Gerd & Kevin, because they were involved with the code that gets
> refactored here, and no good deed shall go unpunished.
> 
> "Wang, Lei" <lei4.wang@intel.com> writes:
> 
>> QEMU use qemu_add_opts() and qemu_add_drive_opts() to add config options
>> when initialization. Extract the same logic in both functions to a
>> seperate function fill_config_groups() to reduce code redundency.
>>
>> Signed-off-by: Wang, Lei <lei4.wang@intel.com>
>> ---
>>  util/qemu-config.c | 39 ++++++++++++++++++++-------------------
>>  1 file changed, 20 insertions(+), 19 deletions(-)
>>
>> diff --git a/util/qemu-config.c b/util/qemu-config.c
>> index 433488aa56..3a1c85223a 100644
>> --- a/util/qemu-config.c
>> +++ b/util/qemu-config.c
>> @@ -282,36 +282,37 @@ QemuOptsList *qemu_find_opts_err(const char *group, Error **errp)
>>      return find_list(vm_config_groups, group, errp);
>>  }
>>  
>> -void qemu_add_drive_opts(QemuOptsList *list)
>> +static int fill_config_groups(QemuOptsList *groups[], int entries,
>> +                              QemuOptsList *list)
>>  {
>> -    int entries, i;
>> +    int i;
>>  
>> -    entries = ARRAY_SIZE(drive_config_groups);
>>      entries--; /* keep list NULL terminated */
>>      for (i = 0; i < entries; i++) {
>> -        if (drive_config_groups[i] == NULL) {
>> -            drive_config_groups[i] = list;
>> -            return;
>> +        if (groups[i] == NULL) {
>> +            groups[i] = list;
>> +            return 0;
>>          }
>>      }
>> -    fprintf(stderr, "ran out of space in drive_config_groups");
>> -    abort();
>> +    return -1;
>>  }
>>  
>> -void qemu_add_opts(QemuOptsList *list)
>> +void qemu_add_drive_opts(QemuOptsList *list)
>>  {
>> -    int entries, i;
>> +    if (fill_config_groups(drive_config_groups, ARRAY_SIZE(drive_config_groups),
>> +                           list) < 0) {
>> +        fprintf(stderr, "ran out of space in drive_config_groups");
>> +        abort();
>> +    }
>> +}
>>  
>> -    entries = ARRAY_SIZE(vm_config_groups);
>> -    entries--; /* keep list NULL terminated */
>> -    for (i = 0; i < entries; i++) {
>> -        if (vm_config_groups[i] == NULL) {
>> -            vm_config_groups[i] = list;
>> -            return;
>> -        }
>> +void qemu_add_opts(QemuOptsList *list)
>> +{
>> +    if (fill_config_groups(vm_config_groups, ARRAY_SIZE(vm_config_groups),
>> +                           list) < 0) {
>> +        fprintf(stderr, "ran out of space in vm_config_groups");
>> +        abort();
>>      }
>> -    fprintf(stderr, "ran out of space in vm_config_groups");
>> -    abort();
>>  }
>>  
>>  /* Returns number of config groups on success, -errno on error */
>