Add some handy string list functions, for general use now, and for
eventual use in the cpr/live update patches.

Steve Sistare (4):
  qapi: strList_from_string
  qapi: QAPI_LIST_LENGTH
  qapi: strv_from_strList
  qapi: strList unit tests

 include/monitor/hmp.h     |  1 -
 include/qapi/util.h       | 28 ++++++++++++++++
 monitor/hmp-cmds.c        | 19 -----------
 net/net-hmp-cmds.c        |  2 +-
 qapi/qapi-util.c          | 37 ++++++++++++++++++++++
 stats/stats-hmp-cmds.c    |  2 +-
 tests/unit/meson.build    |  1 +
 tests/unit/test-strlist.c | 81 +++++++++++++++++++++++++++++++++++++++++++++++
 8 files changed, 149 insertions(+), 22 deletions(-)
 create mode 100644 tests/unit/test-strlist.c

-- 
1.8.3.1

On Tue, Feb 07, 2023 at 10:48:43AM -0800, Steve Sistare wrote:
> Add some handy string list functions, for general use now, and for
> eventual use in the cpr/live update patches.
> 
> Steve Sistare (4):
>   qapi: strList_from_string
>   qapi: QAPI_LIST_LENGTH
>   qapi: strv_from_strList
>   qapi: strList unit tests

I know that the 'strList' type falls out naturally from the
QAPI type generator for arrays, but I've always considered
it to be a rather awkward result.  The normal C approach
would be to use 'char **' NULL terminated, which conveniently
already has a bunch of helper APIs from glib, and is also
accepted or returned by various other functions we might
like to use.

Should we consider making the QAPI generator handle string
lists as a special case, emitting 'char **' instead of this
series ?

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Daniel P. Berrangé <berrange@redhat.com> writes:

> On Tue, Feb 07, 2023 at 10:48:43AM -0800, Steve Sistare wrote:
>> Add some handy string list functions, for general use now, and for
>> eventual use in the cpr/live update patches.
>> 
>> Steve Sistare (4):
>>   qapi: strList_from_string
>>   qapi: QAPI_LIST_LENGTH
>>   qapi: strv_from_strList
>>   qapi: strList unit tests
>
> I know that the 'strList' type falls out naturally from the
> QAPI type generator for arrays, but I've always considered
> it to be a rather awkward result.  The normal C approach
> would be to use 'char **' NULL terminated, which conveniently
> already has a bunch of helper APIs from glib, and is also
> accepted or returned by various other functions we might
> like to use.
>
> Should we consider making the QAPI generator handle string
> lists as a special case, emitting 'char **' instead of this
> series ?

I don't like special cases.  I also don't like GenericList in any case.

I believe a linked list was chosen because it results in a fairly simple
visitor interface and implementation.  But it's a poor data structure
for a homogeneous sequence that rarely if ever changes: lots of pointer
chasing, waste of memory when the elements are small.

Output visitors walk the sequence in order.  An array would be perfect.

Input visitors build the sequence by appending elements in order.
A flexible array like GArray would do.

I'm not aware of other code mutating GenericLists or its descendants.
It might exist.  I'd be surprised if there's much of it, though.

On 2/9/2023 5:48 AM, Daniel P. Berrangé wrote:
> On Tue, Feb 07, 2023 at 10:48:43AM -0800, Steve Sistare wrote:
>> Add some handy string list functions, for general use now, and for
>> eventual use in the cpr/live update patches.
>>
>> Steve Sistare (4):
>>   qapi: strList_from_string
>>   qapi: QAPI_LIST_LENGTH
>>   qapi: strv_from_strList
>>   qapi: strList unit tests
> 
> I know that the 'strList' type falls out naturally from the
> QAPI type generator for arrays, but I've always considered
> it to be a rather awkward result.  The normal C approach
> would be to use 'char **' NULL terminated, which conveniently
> already has a bunch of helper APIs from glib, and is also
> accepted or returned by various other functions we might
> like to use.
> 
> Should we consider making the QAPI generator handle string
> lists as a special case, emitting 'char **' instead of this
> series ?
> 
> With regards,
> Daniel

That is an intellectually appealing idea, but it sounds like a disproportionate 
effort to handle this small use case.  It would also make string list handling
be different than the other qapi lists: boolList, sizeList, uint64List, etc.

- Steve

Steve Sistare <steven.sistare@oracle.com> writes:

> Add some handy string list functions, for general use now, and for
> eventual use in the cpr/live update patches.

Submit them together with uses, please.

On 2/9/2023 5:05 AM, Markus Armbruster wrote:
> Steve Sistare <steven.sistare@oracle.com> writes:
> 
>> Add some handy string list functions, for general use now, and for
>> eventual use in the cpr/live update patches.
> 
> Submit them together with uses, please.

Are you OK with me describing my intended uses in the commit message?  Or are you 
suggesting I go back to submitting these in the larger live update series?

- Steve

Steven Sistare <steven.sistare@oracle.com> writes:

> On 2/9/2023 5:05 AM, Markus Armbruster wrote:
>> Steve Sistare <steven.sistare@oracle.com> writes:
>> 
>>> Add some handy string list functions, for general use now, and for
>>> eventual use in the cpr/live update patches.
>> 
>> Submit them together with uses, please.
>
> Are you OK with me describing my intended uses in the commit message?  Or are you 
> suggesting I go back to submitting these in the larger live update series?

The patches will be merged only together with actual uses.  Posting them
separately may make sense or not; use your judgement.  If you elect to
post separately, have the cover letters point to each other, please.