Message ID | 1675795727-235010-1-git-send-email-steven.sistare@oracle.com |
---|---|
Headers | show |
Series | string list functions | expand |
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 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
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
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
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.
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.