Fix qos_flows confguration reporting (#1577)

mgr:config output contained garbage when qos_flows was not configured.

diff --git a/src/ip/QosConfig.cc b/src/ip/QosConfig.cc
index c9c75b67954698cad92de64f9157cff61b73c43a..20800da3dd2b33adbe62b8a4baafcfdca38ec996 100644 (file)
--- a/src/ip/QosConfig.cc
+++ b/src/ip/QosConfig.cc
@@ -9,6 +9,7 @@
 #include "squid.h"
 #include "acl/Gadgets.h"
 #include "base/IoManip.h"
+#include "base/PackableStream.h"
 #include "cache_cf.h"
 #include "comm/Connection.h"
 #include "compat/cmsg.h"
@@ -19,6 +20,7 @@
 #include "ip/QosConfig.h"
 #include "ip/tools.h"
 #include "Parsing.h"
+#include "Store.h"
 
 #include <cerrno>
 #include <limits>
@@ -459,70 +461,67 @@ Ip::Qos::Config::parseConfigLine()
     }
 }
 
-/**
- * NOTE: Due to the low-level nature of the library these
- * objects are part of the dump function must be self-contained.
- * which means no StoreEntry references. Just a basic char* buffer.
-*/
+/// helper function for printing Ip::Qos::Config mark and tos values
+template <class Integer>
+static auto asQosConfigHex(const Integer n) { return asHex(n).upperCase().minDigits(2); }
+
+/// report configuration using qos_flows syntax
 void
-Ip::Qos::Config::dumpConfigLine(char *entry, const char *name) const
+Ip::Qos::Config::dumpConfigLine(std::ostream &os, const char *directiveName) const
 {
-    char *p = entry;
     if (isHitTosActive()) {
-
-        p += snprintf(p, 11, "%s", name); // strlen("qos_flows ");
-        p += snprintf(p, 4, "%s", "tos");
+        os << directiveName << " tos";
 
         if (tosLocalHit > 0) {
-            p += snprintf(p, 16, " local-hit=0x%02X", tosLocalHit);
+            os << " local-hit=0x" << asQosConfigHex(tosLocalHit);
         }
         if (tosSiblingHit > 0) {
-            p += snprintf(p, 18, " sibling-hit=0x%02X", tosSiblingHit);
+            os << " sibling-hit=0x" << asQosConfigHex(tosSiblingHit);
         }
         if (tosParentHit > 0) {
-            p += snprintf(p, 17, " parent-hit=0x%02X", tosParentHit);
+            os << " parent-hit=0x" << asQosConfigHex(tosParentHit);
         }
         if (tosMiss > 0) {
-            p += snprintf(p, 11, " miss=0x%02X", tosMiss);
+            os << " miss=0x" << asQosConfigHex(tosMiss);
             if (tosMissMask!=0xFFU) {
-                p += snprintf(p, 6, "/0x%02X", tosMissMask);
+                os << "/0x" << asQosConfigHex(tosMissMask);
             }
         }
         if (preserveMissTos == 0) {
-            p += snprintf(p, 23, " disable-preserve-miss");
+            os << " disable-preserve-miss";
         }
         if (preserveMissTos && preserveMissTosMask != 0) {
-            p += snprintf(p, 16, " miss-mask=0x%02X", preserveMissTosMask);
+            os << " miss-mask=0x" << asQosConfigHex(preserveMissTosMask);
         }
-        p += snprintf(p, 2, "\n");
+        os << "\n";
+        return;
     }
 
     if (isHitNfmarkActive()) {
-        p += snprintf(p, 11, "%s", name); // strlen("qos_flows ");
-        p += snprintf(p, 5, "%s", "mark");
+        os << directiveName << " mark";
 
         if (markLocalHit > 0) {
-            p += snprintf(p, 22, " local-hit=0x%02X", markLocalHit);
+            os << " local-hit=0x" << asQosConfigHex(markLocalHit);
         }
         if (markSiblingHit > 0) {
-            p += snprintf(p, 24, " sibling-hit=0x%02X", markSiblingHit);
+            os << " sibling-hit=0x" << asQosConfigHex(markSiblingHit);
         }
         if (markParentHit > 0) {
-            p += snprintf(p, 23, " parent-hit=0x%02X", markParentHit);
+            os << " parent-hit=0x" << asQosConfigHex(markParentHit);
         }
         if (markMiss > 0) {
-            p += snprintf(p, 17, " miss=0x%02X", markMiss);
+            os << " miss=0x" << asQosConfigHex(markMiss);
             if (markMissMask!=0xFFFFFFFFU) {
-                p += snprintf(p, 12, "/0x%02X", markMissMask);
+                os << "/0x" << asQosConfigHex(markMissMask);
             }
         }
         if (preserveMissMark == false) {
-            p += snprintf(p, 23, " disable-preserve-miss");
+            os << " disable-preserve-miss";
         }
         if (preserveMissMark && preserveMissMarkMask != 0) {
-            p += snprintf(p, 22, " miss-mask=0x%02X", preserveMissMarkMask);
+            os << " miss-mask=" << asQosConfigHex(preserveMissMarkMask);
         }
-        p += snprintf(p, 2, "\n");
+        os << "\n";
     }
 }
 
@@ -640,3 +639,9 @@ Ip::Qos::Config::isAclTosActive() const
     return false;
 }
 
+void
+dump_QosConfig(StoreEntry * const entry, const char * const directiveName, const Ip::Qos::Config &config)
+{
+    PackableStream os(*entry);
+    config.dumpConfigLine(os, directiveName);
+}

diff --git a/src/ip/QosConfig.h b/src/ip/QosConfig.h
index 613aa0a19868e243d117b69cc09023e96c2480da..37f6eff68ade9d778504744d62f959fa7198107a 100644 (file)
--- a/src/ip/QosConfig.h
+++ b/src/ip/QosConfig.h
@@ -15,6 +15,7 @@
 #include "hier_code.h"
 #include "ip/forward.h"
 #include "ip/NfMarkConfig.h"
+#include "store/forward.h"
 
 #if HAVE_LIBNETFILTER_CONNTRACK_LIBNETFILTER_CONNTRACK_H
 #include <libnetfilter_conntrack/libnetfilter_conntrack.h>
@@ -22,6 +23,7 @@
 #if HAVE_LIBNETFILTER_CONNTRACK_LIBNETFILTER_CONNTRACK_TCP_H
 #include <libnetfilter_conntrack/libnetfilter_conntrack_tcp.h>
 #endif
+#include <iosfwd>
 #include <limits>
 
 class fde;
@@ -187,7 +189,7 @@ public:
      * objects are part of the dump function must be self-contained.
      * which means no StoreEntry references. Just a basic char* buffer.
      */
-    void dumpConfigLine(char *entry, const char *name) const;
+    void dumpConfigLine(std::ostream &, const char *) const;
 
     /// Whether we should modify TOS flags based on cache hits and misses.
     bool isHitTosActive() const {
@@ -240,18 +242,14 @@ public:
 /// Globally available instance of Qos::Config
 extern Config TheConfig;
 
-/* legacy parser access wrappers */
-#define parse_QosConfig(X)  (X)->parseConfigLine()
-#define free_QosConfig(X)
-#define dump_QosConfig(e,n,X) do { \
-        char temp[256]; /* random number. change as needed. max config line length. */ \
-        (X).dumpConfigLine(temp,n); \
-            storeAppendPrintf(e, "%s", temp); \
-    } while(0);
-
 } // namespace Qos
 
 } // namespace Ip
 
+/* legacy parser access wrappers */
+inline void parse_QosConfig(Ip::Qos::Config * c) { c->parseConfigLine(); }
+inline void free_QosConfig(Ip::Qos::Config *) {}
+void dump_QosConfig(StoreEntry *, const char * directiveName, const Ip::Qos::Config &);
+
 #endif /* SQUID_QOSCONFIG_H */
 

diff --git a/src/tests/stub_libip.cc b/src/tests/stub_libip.cc
index 0616767ddafafe076663c6c69ffce58cf9a012d8..b46ba075fc292a566cb4c3651b66d0519ee92ed6 100644 (file)
--- a/src/tests/stub_libip.cc
+++ b/src/tests/stub_libip.cc
@@ -91,7 +91,7 @@ int Ip::Qos::setSockNfmark(const Comm::ConnectionPointer &, nfmark_t) STUB_RETVA
 int Ip::Qos::setSockNfmark(const int, nfmark_t) STUB_RETVAL(-1)
 Ip::Qos::Config::Config() STUB_NOP
 void Ip::Qos::Config::parseConfigLine() STUB
-void Ip::Qos::Config::dumpConfigLine(char *, const char *) const STUB
+void Ip::Qos::Config::dumpConfigLine(std::ostream &, const char *) const STUB
 bool Ip::Qos::Config::isAclNfmarkActive() const STUB_RETVAL(false)
 bool Ip::Qos::Config::isAclTosActive() const STUB_RETVAL(false)
 Ip::Qos::Config Ip::Qos::TheConfig;