Clean up QXmlTestLogger's xmlQuote() and xmlCdata()

Make the overloads taking a size private (they're only called by the
implementation of the others) and have them take qsizetype for the
size, rather than size_t. Make the still-public ones consistently
return 0 on failure and move the documentation to them.

Change some spurious checks (QTestCharBuffer::size() is necessarily
positive) to assertions, use '\0' for the nul character to make it
visually distinct from the integer 0. Use a do..while loop instead of
break-ing out of the tail of a for (;;) loop. Remove spurious extra
nul-termination - the function taking size alread did that, so the
wrapper to size-expand needn't. Minor style cleanups.

Change-Id: I7440f3d9be5e3b2dd88c2aad62bcd1af9fc02278
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
Reviewed-by: Tor Arne Vestbø <tor.arne.vestbo@qt.io>

2 files changed, 63 insertions, 63 deletions

diff --git a/src/testlib/qxmltestlogger.cpp b/src/testlib/qxmltestlogger.cpp
index 9349a56a11..716a99d80d 100644
--- a/src/testlib/qxmltestlogger.cpp
+++ b/src/testlib/qxmltestlogger.cpp
@@ -1,6 +1,6 @@
 /****************************************************************************
 **
-** Copyright (C) 2016 The Qt Company Ltd.
+** Copyright (C) 2021 The Qt Company Ltd.
 ** Contact: https://www.qt.io/licensing/
 **
 ** This file is part of the QtTest module of the Qt Toolkit.
@@ -308,18 +308,15 @@ void QXmlTestLogger::addMessage(MessageTypes type, const QString &message,
     outputString(buf.constData());
 }
 
-/*
-    Copy up to n characters from the src string into dest, escaping any special
-    XML characters as necessary so that dest is suitable for use in an XML
-    quoted attribute string.
-*/
-int QXmlTestLogger::xmlQuote(QTestCharBuffer *destBuf, char const *src, size_t n)
+int QXmlTestLogger::xmlQuote(QTestCharBuffer *destBuf, char const *src, qsizetype n)
 {
-    if (n == 0) return 0;
+    Q_ASSERT(n > 0 && destBuf->size() >= n);
 
     char *dest = destBuf->data();
-    *dest = 0;
-    if (!src) return 0;
+    if (!src || n == 1) {
+        *dest = '\0';
+        return 0;
+    }
 
     char *begin = dest;
     char *end = dest + n;
@@ -332,10 +329,9 @@ int QXmlTestLogger::xmlQuote(QTestCharBuffer *destBuf, char const *src, size_t n
             if (dest + sizeof(ent) < end) {         \
                 strcpy(dest, ent);                  \
                 dest += sizeof(ent) - 1;            \
-            }                                       \
-            else {                                  \
-                *dest = 0;                          \
-                return (dest+sizeof(ent)-begin);    \
+            } else {                                \
+                *dest = '\0';                       \
+                return dest + sizeof(ent) - begin;  \
             }                                       \
             ++src;                                  \
             break;
@@ -346,15 +342,15 @@ int QXmlTestLogger::xmlQuote(QTestCharBuffer *destBuf, char const *src, size_t n
             MAP_ENTITY('"', "&quot;");
             MAP_ENTITY('&', "&amp;");
 
-            // not strictly necessary, but allows handling of comments without
+            // Not strictly necessary, but allows handling of comments without
             // having to explicitly look for `--'
             MAP_ENTITY('-', "&#x002D;");
 
 #undef MAP_ENTITY
 
-        case 0:
-            *dest = 0;
-            return (dest-begin);
+        case '\0':
+            *dest = '\0';
+            return dest - begin;
 
         default:
             *dest = *src;
@@ -365,45 +361,41 @@ int QXmlTestLogger::xmlQuote(QTestCharBuffer *destBuf, char const *src, size_t n
     }
 
     // If we get here, dest was completely filled (dest == end)
-    *(dest-1) = 0;
-    return (dest-begin);
+    dest[-1] = '\0';
+    return dest - begin;
 }
 
-/*
-    Copy up to n characters from the src string into dest, escaping any
-    special strings such that dest is suitable for use in an XML CDATA section.
-*/
-int QXmlTestLogger::xmlCdata(QTestCharBuffer *destBuf, char const *src, size_t n)
+int QXmlTestLogger::xmlCdata(QTestCharBuffer *destBuf, char const *src, qsizetype n)
 {
-    if (!n) return 0;
+    Q_ASSERT(n > 0 && destBuf->size() >= n);
 
     char *dest = destBuf->data();
 
     if (!src || n == 1) {
-        *dest = 0;
+        *dest = '\0';
         return 0;
     }
 
     static char const CDATA_END[] = "]]>";
     static char const CDATA_END_ESCAPED[] = "]]]><![CDATA[]>";
+    const size_t CDATA_END_LEN = sizeof(CDATA_END) - 1;
 
     char *begin = dest;
     char *end = dest + n;
     while (dest < end) {
         if (!*src) {
-            *dest = 0;
-            return (dest-begin);
+            *dest = '\0';
+            return dest - begin;
         }
 
-        if (!strncmp(src, CDATA_END, sizeof(CDATA_END)-1)) {
+        if (!strncmp(src, CDATA_END, CDATA_END_LEN)) {
             if (dest + sizeof(CDATA_END_ESCAPED) < end) {
                 strcpy(dest, CDATA_END_ESCAPED);
-                src += sizeof(CDATA_END)-1;
+                src += CDATA_END_LEN;
                 dest += sizeof(CDATA_END_ESCAPED) - 1;
-            }
-            else {
-                *dest = 0;
-                return (dest+sizeof(CDATA_END_ESCAPED)-begin);
+            } else {
+                *dest = '\0';
+                return dest + sizeof(CDATA_END_ESCAPED) - begin;
             }
             continue;
         }
@@ -413,12 +405,13 @@ int QXmlTestLogger::xmlCdata(QTestCharBuffer *destBuf, char const *src, size_t n
         ++dest;
     }
 
-    // If we get here, dest was completely filled (dest == end)
-    *(dest-1) = 0;
-    return (dest-begin);
+    // If we get here, dest was completely filled:
+    Q_ASSERT(dest == end && end > begin);
+    dest[-1] = '\0';
+    return dest - begin;
 }
 
-typedef int (*StringFormatFunction)(QTestCharBuffer *, char const *, size_t);
+typedef int (*StringFormatFunction)(QTestCharBuffer *, char const *, qsizetype);
 
 /*
     A wrapper for string functions written to work with a fixed size buffer so they can be called
@@ -426,36 +419,43 @@ typedef int (*StringFormatFunction)(QTestCharBuffer *, char const *, size_t);
 */
 int allocateStringFn(QTestCharBuffer *str, char const *src, StringFormatFunction func)
 {
-    static const int MAXSIZE = 1024*1024*2;
-
+    constexpr int MAXSIZE = 1024 * 1024 * 2;
     int size = str->size();
 
-    int res = 0;
+    do {
+        const int res = func(str, src, size);
+        if (res < size) // Success or fatal failure
+            return res;
 
-    for (;;) {
-        res = func(str, src, size);
-        str->data()[size - 1] = '\0';
-        if (res < size) {
-            // We succeeded or fatally failed
-            break;
-        }
-        // buffer wasn't big enough, try again
+        // Buffer wasn't big enough, try again, if not too big:
         size *= 2;
-        if (size > MAXSIZE) {
-            break;
-        }
-        if (!str->reset(size))
-            break; // ran out of memory - bye
-    }
+    } while (size <= MAXSIZE && str->reset(size));
 
-    return res;
+    return 0;
 }
 
+/*
+    Copy from \a src into \a destBuf, escaping any special XML characters as
+    necessary so that destBuf is suitable for use in an XML quoted attribute
+    string. Expands \a destBuf as needed to make room, up to a size of 2
+    MiB. Input requiring more than that much space for output is considered
+    invalid.
+
+    Returns 0 on invalid or empty input, the actual length written on success.
+*/
 int QXmlTestLogger::xmlQuote(QTestCharBuffer *str, char const *src)
 {
     return allocateStringFn(str, src, QXmlTestLogger::xmlQuote);
 }
 
+/*
+    Copy from \a src into \a destBuf, escaping any special strings such that
+    destBuf is suitable for use in an XML CDATA section. Expands \a destBuf as
+    needed to make room, up to a size of 2 MiB. Input requiring more than that
+    much space for output is considered invalid.
+
+    Returns 0 on invalid or empty input, the actual length written on success.
+*/
 int QXmlTestLogger::xmlCdata(QTestCharBuffer *str, char const *src)
 {
     return allocateStringFn(str, src, QXmlTestLogger::xmlCdata);
diff --git a/src/testlib/qxmltestlogger_p.h b/src/testlib/qxmltestlogger_p.h
index 04ed57d587..8091dada59 100644
--- a/src/testlib/qxmltestlogger_p.h
+++ b/src/testlib/qxmltestlogger_p.h
@@ -1,6 +1,6 @@
 /****************************************************************************
 **
-** Copyright (C) 2016 The Qt Company Ltd.
+** Copyright (C) 2021 The Qt Company Ltd.
 ** Contact: https://www.qt.io/licensing/
 **
 ** This file is part of the QtTest module of the Qt Toolkit.
@@ -77,12 +77,12 @@ public:
     void addMessage(MessageTypes type, const QString &message,
                     const char *file = nullptr, int line = 0) override;
 
-    static int xmlCdata(QTestCharBuffer *dest, char const* src);
-    static int xmlQuote(QTestCharBuffer *dest, char const* src);
-    static int xmlCdata(QTestCharBuffer *dest, char const* src, size_t n);
-    static int xmlQuote(QTestCharBuffer *dest, char const* src, size_t n);
-
+    static int xmlCdata(QTestCharBuffer *dest, char const *src);
+    static int xmlQuote(QTestCharBuffer *dest, char const *src);
 private:
+    static int xmlCdata(QTestCharBuffer *dest, char const *src, qsizetype n);
+    static int xmlQuote(QTestCharBuffer *dest, char const *src, qsizetype n);
+
     XmlMode xmlmode;
 };