Re: [libvirt] [PATCH 05/11] util: use glib string allocation/formatting functions

On Fri, Sep 27, 2019 at 06:17:27PM +0100, Daniel P. Berrangé wrote: > Convert the string duplication APIs to use the g_strdup family of APIs. > > Annoyingly our virVasprintf/virAsprintf functions return the character > count, even though 90% of our usage doesn't need it. To retain compat > with these semantics we have a call to strlen which costs CPU time. > > We previously used the 'strdup-posix' gnulib module because mingw does > not set errno to ENOMEM on failure > > We previously used the 'strndup' gnulib module because this function > does not exist on mingw. > > We previously used the 'vasprintf' gnulib module because of many GNU > supported format specifiers not working on non-Linux platforms. glib's > own equivalent standardizes on GNU format specifiers too. > > Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; > --- > bootstrap.conf | 3 --- > src/util/virstring.c | 19 +++++++------------ > 2 files changed, 7 insertions(+), 15 deletions(-) > > diff --git a/bootstrap.conf b/bootstrap.conf > index 549d18c6d4..b6b75f9301 100644 > --- a/bootstrap.conf > +++ b/bootstrap.conf > @@ -100,8 +100,6 @@ stat-time > stdarg > stpcpy > strchrnul > -strdup-posix > -strndup > strerror > strerror_r-posix > strptime > @@ -117,7 +115,6 @@ uname > unsetenv > useless-if-before-free > usleep > -vasprintf > verify > vc-list-files > vsnprintf > diff --git a/src/util/virstring.c b/src/util/virstring.c > index a4cc7e9c0a..c8c888b2a0 100644 > --- a/src/util/virstring.c > +++ b/src/util/virstring.c > @@ -730,12 +730,9 @@ virVasprintfInternal(char **strp, > const char *fmt, > va_list list) > { > - int ret; > + *strp = g_strdup_vprintf(fmt, list); > > - if ((ret = vasprintf(strp, fmt, list)) == -1) > - abort(); > - > - return ret; > + return strlen(*strp); This will cause a SEGFAULT if strp is NULL as g_strdup_vprintf doesn't abort on failure. We can use g_vasprintf which returns length.

But if we want to return only -1 or 0 and let the caller to decide on the length there are only few places to modify. src/nwfilter/nwfilter_dhcpsnoop.c:1770 src/util/virfile.c:3410 These two looks like the only cases where we actually care about the length. There are some other cases for which we would have to only tweak to comparison: src/libxl/libxl_domain.c:916: There is a function virDoubleToStr that returns the length but it's usage doesn't care about the length so we would have to change the description.