Re: [libvirt] [PATCH v2 1/4] util: string: Introduce macro for automatic string lists

On Wed, Feb 27, 2019 at 10:52:47 +0100, Erik Skultety wrote: > On Tue, Feb 26, 2019 at 04:48:23PM +0100, Peter Krempa wrote: >> Similar to VIR_AUTOPTR, VIR_AUTOSTRINGLIST defines a list of strings >> which will be freed if the pointer is leaving scope. >> >> Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; >> --- >> src/libvirt_private.syms | 1 + >> src/util/virstring.c | 10 ++++++++++ >> src/util/virstring.h | 10 ++++++++++ >> 3 files changed, 21 insertions(+) >> >> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms >> index 038a744981..e68e3f3a3b 100644 >> --- a/src/libvirt_private.syms >> +++ b/src/libvirt_private.syms >> @@ -2958,6 +2958,7 @@ virStringHasControlChars; >> virStringIsEmpty; >> virStringIsPrintable; >> virStringListAdd; >> +virStringListAutoFree; >> virStringListFree; >> virStringListFreeCount; >> virStringListGetFirstWithPrefix; >> diff --git a/src/util/virstring.c b/src/util/virstring.c >> index e890dde546..8a791f96d4 100644 >> --- a/src/util/virstring.c >> +++ b/src/util/virstring.c >> @@ -318,6 +318,16 @@ void virStringListFree(char **strings) >> } >> >> >> +void virStringListAutoFree(char ***strings) >> +{ >> + if (!*strings) >> + return; >> + >> + virStringListFree(*strings); >> + *strings = NULL; >> +} >> + >> + >> /** >> * virStringListFreeCount: >> * @strings: array of strings to free >> diff --git a/src/util/virstring.h b/src/util/virstring.h >> index aef82471c2..d14b7f4f49 100644 >> --- a/src/util/virstring.h >> +++ b/src/util/virstring.h >> @@ -53,6 +53,7 @@ int virStringListCopy(char ***dst, >> const char **src); >> >> void virStringListFree(char **strings); >> +void virStringListAutoFree(char ***strings); >> void virStringListFreeCount(char **strings, >> size_t count); >> >> @@ -307,6 +308,15 @@ int virStringParsePort(const char *str, >> unsigned int *port) >> ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; >> >> +/** >> + * VIR_AUTOSTRINGLIST: >> + * >> + * Declares a NULL-terminated list of strings which will be automatically freed >> + * when the pointer goes out of scope. >> + */ >> +# define VIR_AUTOSTRINGLIST \ >> + __attribute__((cleanup(virStringListAutoFree))) char ** >> + > > IIRC at the beginning we said that all the VIR_AUTO macros should be consistent > in terms of how we use the macro, IOW: > > VIR_AUTOFREE(type) > VIR_AUTOPTR(type) > VIR_AUTOCLEAN(type) > > although I can't say I personally don't like your version, nevertheless, as I > said we should remain consistent and use 'char **' explicitly when using the > macro. I'm not sure I understood. Did you mean that it should be used as: VIR_AUTOSTRINGLIST char **list = NULL; ?