On Mon, Sep 30, 2019 at 12:42:56PM +0100, Daniel P. Berrangé wrote:
On Mon, Sep 30, 2019 at 01:35:36PM +0200, Pavel Hrdina wrote: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@xxxxxxxxxx> > --- > 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.Oh yes, that makes life easier.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
[0]
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.There something else hiding as we break all test suite output if we return 0
Yes, that's the virFilePrintf function Pavel pointed out above [0] Jano
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 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
