[libvirt] [PATCH 01/23] util: Introduce virStringListCopy

[Jiri Denemark]

On Thu, Oct 12, 2017 at 07:22:51 -0400, John Ferlan wrote:
> 
> 
> On 10/04/2017 10:58 AM, Jiri Denemark wrote:
> > The API makes a deep copy of a NULL-terminated string list.
> > 
> > Signed-off-by: Jiri Denemark <jdenemar at redhat.com>
> > ---
> >  src/util/virstring.c | 37 +++++++++++++++++++++++++++++++++++++
> >  src/util/virstring.h |  3 +++
> >  2 files changed, 40 insertions(+)
> > 
> > diff --git a/src/util/virstring.c b/src/util/virstring.c
> > index 0288d1e677..820b282ac5 100644
> > --- a/src/util/virstring.c
> > +++ b/src/util/virstring.c
> > @@ -239,6 +239,43 @@ virStringListRemove(char ***strings,
> >  }
> >  
> >  
> > +/**
> > + * virStringListCopy:
> > + * @dst: where to store the copy of @strings
> > + * @src: a NULL-terminated array of strings
> > + *
> > + * Makes a deep copy of the @src string list and stores it in @dst. Callers
> > + * are responsible for freeing both @dst and @src.
> > + *
> > + * Returns 0 on success, -1 on error.
> > + */
> > +int
> > +virStringListCopy(char ***dst,
> > +                  const char **src)
> > +{
> 
> I think it would make more sense to have this return @copy (or call it
> @dst, doesn't matter) rather than 0, -1 which only means @dst wasn't
> populated.  There's only 1 consumer (in patch 2)...

Returning the pointer rather than int makes it impossible to allow NULL
input since returning NULL would mean something failed. This is similar
to VIR_STRDUP and several others.

Jirka