In a few places our code relies on the fact that virAsprintf()
not only prints to allocated string but also that it returns the
length of that string. Fortunately, only few such places were
identified:

  https://www.redhat.com/archives/libvir-list/2019-September/msg01382.html

In case of virNWFilterSnoopLeaseFileWrite() and virFilePrintf()
we can use strlen() right after virAsprintf() to calculate the
length. In case of virDoubleToStr() it's only caller checks for
error case only, so we can limit the set of returned values to
just [-1, 0].

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/nwfilter/nwfilter_dhcpsnoop.c |  7 +++----
 src/util/virfile.c                |  5 +++--
 src/util/virstring.c              | 10 ++++++----
 3 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/src/nwfilter/nwfilter_dhcpsnoop.c b/src/nwfilter/nwfilter_dhcpsnoop.c
index bf25345e2c..665e7bff9c 100644
--- a/src/nwfilter/nwfilter_dhcpsnoop.c
+++ b/src/nwfilter/nwfilter_dhcpsnoop.c
@@ -1766,13 +1766,12 @@ virNWFilterSnoopLeaseFileWrite(int lfd, const char *ifkey,
     }
 
     /* time intf ip dhcpserver */
-    len = virAsprintf(&lbuf, "%u %s %s %s\n", ipl->timeout,
-                      ifkey, ipstr, dhcpstr);
-
-    if (len < 0) {
+    if (virAsprintf(&lbuf, "%u %s %s %s\n", ipl->timeout,
+                    ifkey, ipstr, dhcpstr) < 0) {
         ret = -1;
         goto cleanup;
     }
+    len = strlen(lbuf);
 
     if (safewrite(lfd, lbuf, len) != len) {
         virReportSystemError(errno, "%s", _("lease file write failed"));
diff --git a/src/util/virfile.c b/src/util/virfile.c
index c4d544be73..7c97317994 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -3394,12 +3394,13 @@ int virFilePrintf(FILE *fp, const char *msg, ...)
 {
     va_list vargs;
     g_autofree char *str = NULL;
-    int ret;
+    int ret = -1;
 
     va_start(vargs, msg);
 
-    if ((ret = virVasprintf(&str, msg, vargs)) < 0)
+    if (virVasprintf(&str, msg, vargs) < 0)
         goto cleanup;
+    ret = strlen(str);
 
     if (fwrite(str, 1, ret, fp) != ret) {
         virReportSystemError(errno, "%s",
diff --git a/src/util/virstring.c b/src/util/virstring.c
index 1494eab132..4294b7456e 100644
--- a/src/util/virstring.c
+++ b/src/util/virstring.c
@@ -703,23 +703,25 @@ virStrToDouble(char const *s,
  *
  * converts double to string with C locale (thread-safe).
  *
- * Returns -1 on error, size of the string otherwise.
+ * Returns: on success, -1 otherwise.
  */
 int
 virDoubleToStr(char **strp, double number)
 {
     virLocale oldlocale;
-    int ret = -1;
+    int rc = -1;
 
     if (virLocaleSetRaw(&oldlocale) < 0)
         return -1;
 
-    ret = virAsprintf(strp, "%lf", number);
+    rc = virAsprintf(strp, "%lf", number);
 
     virLocaleRevert(&oldlocale);
     virLocaleFixupRadix(strp);
 
-    return ret;
+    if (rc < 0)
+        return -1;
+    return 0;
 }
 
 
-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Tue, Oct 22, 2019 at 03:57:05PM +0200, Michal Privoznik wrote:
>In a few places our code relies on the fact that virAsprintf()
>not only prints to allocated string but also that it returns the
>length of that string. Fortunately, only few such places were
>identified:
>
>  https://www.redhat.com/archives/libvir-list/2019-September/msg01382.html
>
>In case of virNWFilterSnoopLeaseFileWrite() and virFilePrintf()
>we can use strlen() right after virAsprintf() to calculate the
>length. In case of virDoubleToStr() it's only caller checks for
>error case only, so we can limit the set of returned values to
>just [-1, 0].
>
>Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>---
> src/nwfilter/nwfilter_dhcpsnoop.c |  7 +++----
> src/util/virfile.c                |  5 +++--
> src/util/virstring.c              | 10 ++++++----
> 3 files changed, 12 insertions(+), 10 deletions(-)
>diff --git a/src/util/virstring.c b/src/util/virstring.c
>index 1494eab132..4294b7456e 100644
>--- a/src/util/virstring.c
>+++ b/src/util/virstring.c
>@@ -703,23 +703,25 @@ virStrToDouble(char const *s,
>  *
>  * converts double to string with C locale (thread-safe).
>  *
>- * Returns -1 on error, size of the string otherwise.
>+ * Returns: on success, -1 otherwise.

0 on success

>  */
> int
> virDoubleToStr(char **strp, double number)
> {
>     virLocale oldlocale;
>-    int ret = -1;
>+    int rc = -1;

int rc;

I don't think initialization is needed here.

>
>     if (virLocaleSetRaw(&oldlocale) < 0)
>         return -1;
>
>-    ret = virAsprintf(strp, "%lf", number);
>+    rc = virAsprintf(strp, "%lf", number);
>
>     virLocaleRevert(&oldlocale);
>     virLocaleFixupRadix(strp);
>
>-    return ret;
>+    if (rc < 0)
>+        return -1;
>+    return 0;
> }

Reviewed-by: Ján Tomko <jtomko@redhat.com>

Jano
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list