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

[Ján Tomko]

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 at redhat.com>
>> > ---
>> >  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 at redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20190930/f0685b99/attachment-0001.sig>