GCC complains that inlining virStringTrimOptionalNewline is not
likely on some platforms:

  cc1: warnings being treated as errors
  ../../src/util/virfile.c: In function 'virFileReadValueBitmap':
  ../../src/util/virstring.h:292: error: inlining failed in call to 'virStringTrimOptionalNewline': call is unlikely and code size would grow [-Winline]
  ../../src/util/virfile.c:3987: error: called from here [-Winline]

Inlining this function is not going to be a measurable performance
benefit either, since the time required to execute it is going to
be dominated by running of strlen() over the string, not by the
function call overhead.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---

Pushed as a broken build fix

 src/libvirt_private.syms |  1 +
 src/util/virstring.c     | 14 ++++++++++++++
 src/util/virstring.h     |  8 +-------
 3 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 4cca0ca..afb9100 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2630,6 +2630,7 @@ virStringSplitCount;
 virStringStripControlChars;
 virStringStripIPv6Brackets;
 virStringToUpper;
+virStringTrimOptionalNewline;
 virStrncpy;
 virStrndup;
 virStrToDouble;
diff --git a/src/util/virstring.c b/src/util/virstring.c
index 69abc26..335e773 100644
--- a/src/util/virstring.c
+++ b/src/util/virstring.c
@@ -1180,3 +1180,17 @@ virStringEncodeBase64(const uint8_t *buf, size_t buflen)
 
     return ret;
 }
+
+/**
+ * virStringTrimOptionalNewline:
+ * @str: the string to modify in-place
+ *
+ * Modify @str to remove a single '\n' character
+ * from its end, if one exists.
+ */
+void virStringTrimOptionalNewline(char *str)
+{
+    char *tmp = str + strlen(str) - 1;
+    if (*tmp == '\n')
+        *tmp = '\0';
+}
diff --git a/src/util/virstring.h b/src/util/virstring.h
index 603650a..c545ca3 100644
--- a/src/util/virstring.h
+++ b/src/util/virstring.h
@@ -288,12 +288,6 @@ bool virStringBufferIsPrintable(const uint8_t *buf, size_t buflen);
 
 char *virStringEncodeBase64(const uint8_t *buf, size_t buflen);
 
-static inline void
-virStringTrimOptionalNewline(char *str)
-{
-    char *tmp = str + strlen(str) - 1;
-    if (*tmp == '\n')
-        *tmp = '\0';
-}
+void virStringTrimOptionalNewline(char *str);
 
 #endif /* __VIR_STRING_H__ */
-- 
2.9.3

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

On Wed, May 10, 2017 at 09:26:18AM +0100, Daniel P. Berrange wrote:
>GCC complains that inlining virStringTrimOptionalNewline is not
>likely on some platforms:
>
>  cc1: warnings being treated as errors
>  ../../src/util/virfile.c: In function 'virFileReadValueBitmap':
>  ../../src/util/virstring.h:292: error: inlining failed in call to 'virStringTrimOptionalNewline': call is unlikely and code size would grow [-Winline]
>  ../../src/util/virfile.c:3987: error: called from here [-Winline]
>
>Inlining this function is not going to be a measurable performance
>benefit either, since the time required to execute it is going to
>be dominated by running of strlen() over the string, not by the
>function call overhead.
>

Oh, definitely.  Just out of curiosity, though; what platform does this
happen on?
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Wed, May 10, 2017 at 11:12:00AM +0200, Martin Kletzander wrote:
> On Wed, May 10, 2017 at 09:26:18AM +0100, Daniel P. Berrange wrote:
> > GCC complains that inlining virStringTrimOptionalNewline is not
> > likely on some platforms:
> > 
> >  cc1: warnings being treated as errors
> >  ../../src/util/virfile.c: In function 'virFileReadValueBitmap':
> >  ../../src/util/virstring.h:292: error: inlining failed in call to 'virStringTrimOptionalNewline': call is unlikely and code size would grow [-Winline]
> >  ../../src/util/virfile.c:3987: error: called from here [-Winline]
> > 
> > Inlining this function is not going to be a measurable performance
> > benefit either, since the time required to execute it is going to
> > be dominated by running of strlen() over the string, not by the
> > function call overhead.
> > 
> 
> Oh, definitely.  Just out of curiosity, though; what platform does this
> happen on?

I saw it fail on centos6 in CI

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@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list