Fix comments

Signed-off-by: Ze Gan <[email protected]>
sonic-net · Jul 7, 2023 · bcf697e · bcf697e
1 parent 466a131
commit bcf697e
Show file tree

Hide file tree

Showing 8 changed files with 88 additions and 106 deletions.
diff --git a/common/Makefile.am b/common/Makefile.am
@@ -35,6 +35,7 @@ common_libswsscommon_la_SOURCES = \
     common/producerstatetable.cpp    \
     common/zmqproducerstatetable.cpp \
     common/rediscommand.cpp          \
+    common/redisstring.cpp           \
     common/redistran.cpp             \
     common/redisselect.cpp           \
     common/select.cpp                \

diff --git a/common/luatable.cpp b/common/luatable.cpp
@@ -54,15 +54,9 @@ bool LuaTable::get(const vector<string> &luaKeys, vector<FieldValueTuple> &value
         args.emplace_back(v);
     }
 
-    // Transform data structure
-    vector<const char *> args1;
-    transform(args.begin(), args.end(), back_inserter(args1), [](const string &s) { return s.c_str(); } );
-    vector<size_t> lengths;
-    transform(args.begin(), args.end(), back_inserter(lengths), [](const string &s) { return s.length(); } );
-
     // Invoke redis command
     RedisCommand command;
-    command.formatArgv((int)args1.size(), &args1[0], lengths.data());
+    command.format(args);
     RedisReply r(m_db.get(), command, REDIS_REPLY_ARRAY);
     redisReply *reply = r.getContext();
 
@@ -111,15 +105,9 @@ bool LuaTable::hget(const vector<string> &luaKeys, const string &field, string &
         args.emplace_back(v);
     }
 
-    // Transform data structure
-    vector<const char *> args1;
-    transform(args.begin(), args.end(), back_inserter(args1), [](const string &s) { return s.c_str(); } );
-    vector<size_t> lengths;
-    transform(args.begin(), args.end(), back_inserter(lengths), [](const string &s) { return s.length(); } );
-
     // Invoke redis command
     RedisCommand command;
-    command.formatArgv((int)args1.size(), &args1[0], lengths.data());
+    command.format(args);
     RedisReply r(m_db.get(), command);
     redisReply *reply = r.getContext();
 

diff --git a/common/producerstatetable.cpp b/common/producerstatetable.cpp
@@ -134,15 +134,9 @@ void ProducerStateTable::set(const string &key, const vector<FieldValueTuple> &v
         args.emplace_back(fvValue(iv));
     }
 
-    // Transform data structure
-    vector<const char *> args1;
-    transform(args.begin(), args.end(), back_inserter(args1), [](const string &s) { return s.c_str(); } );
-    vector<size_t> lengths;
-    transform(args.begin(), args.end(), back_inserter(lengths), [](const string &s) { return s.length(); } );
-
     // Invoke redis command
     RedisCommand command;
-    command.formatArgv((int)args1.size(), &args1[0], lengths.data());
+    command.format(args);
     m_pipe->push(command, REDIS_REPLY_NIL);
     if (!m_buffered)
     {
@@ -173,15 +167,9 @@ void ProducerStateTable::del(const string &key, const string &op /*= DEL_COMMAND
     args.emplace_back("''");
     args.emplace_back("''");
 
-    // Transform data structure
-    vector<const char *> args1;
-    transform(args.begin(), args.end(), back_inserter(args1), [](const string &s) { return s.c_str(); } );
-    vector<size_t> lengths;
-    transform(args.begin(), args.end(), back_inserter(lengths), [](const string &s) { return s.length(); } );
-
     // Invoke redis command
     RedisCommand command;
-    command.formatArgv((int)args1.size(), &args1[0], lengths.data());
+    command.format(args);
     m_pipe->push(command, REDIS_REPLY_NIL);
     if (!m_buffered)
     {
@@ -228,15 +216,9 @@ void ProducerStateTable::set(const std::vector<KeyOpFieldsValuesTuple>& values)
         }
     }
 
-    // Transform data structure
-    vector<const char *> args1;
-    transform(args.begin(), args.end(), back_inserter(args1), [](const string &s) { return s.c_str(); } );
-    vector<size_t> lengths;
-    transform(args.begin(), args.end(), back_inserter(lengths), [](const string &s) { return s.length(); } );
-
     // Invoke redis command
     RedisCommand command;
-    command.formatArgv((int)args1.size(), &args1[0], lengths.data());
+    command.format(args);
     m_pipe->push(command, REDIS_REPLY_NIL);
     if (!m_buffered)
     {
@@ -271,15 +253,9 @@ void ProducerStateTable::del(const std::vector<std::string>& keys)
     }
     args.emplace_back("G");
 
-    // Transform data structure
-    vector<const char *> args1;
-    transform(args.begin(), args.end(), back_inserter(args1), [](const string &s) { return s.c_str(); } );
-    vector<size_t> lengths;
-    transform(args.begin(), args.end(), back_inserter(lengths), [](const string &s) { return s.length(); } );
-
     // Invoke redis command
     RedisCommand command;
-    command.formatArgv((int)args1.size(), &args1[0], lengths.data());
+    command.format(args);
     m_pipe->push(command, REDIS_REPLY_NIL);
     if (!m_buffered)
     {
@@ -315,15 +291,9 @@ void ProducerStateTable::clear()
     args.emplace_back(getStateHashPrefix() + getTableName());
     args.emplace_back(getDelKeySetName());
 
-    // Transform data structure
-    vector<const char *> args1;
-    transform(args.begin(), args.end(), back_inserter(args1), [](const string &s) { return s.c_str(); } );
-    vector<size_t> lengths;
-    transform(args.begin(), args.end(), back_inserter(lengths), [](const string &s) { return s.length(); } );
-
     // Invoke redis command
     RedisCommand cmd;
-    cmd.formatArgv((int)args1.size(), &args1[0], lengths.data());
+    cmd.format(args);
     m_pipe->push(cmd, REDIS_REPLY_NIL);
     m_pipe->flush();
 }
@@ -476,15 +446,9 @@ void ProducerStateTable::apply_temp_view()
         SWSS_LOG_DEBUG("apply_view.lua is called with following argument list: %s", ss.str().c_str());
     }
 
-    // Transform data structure
-    vector<const char *> args1;
-    transform(args.begin(), args.end(), back_inserter(args1), [](const string &s) { return s.c_str(); } );
-    vector<size_t> lengths;
-    transform(args.begin(), args.end(), back_inserter(lengths), [](const string &s) { return s.length(); } );
-
     // Invoke redis command
     RedisCommand command;
-    command.formatArgv((int)args1.size(), &args1[0], lengths.data());
+    command.format(args);
     m_pipe->push(command, REDIS_REPLY_NIL);
     m_pipe->flush();
 

diff --git a/common/redisapi.h b/common/redisapi.h
@@ -83,22 +83,8 @@ static inline std::set<std::string> runRedisScript(RedisContext &ctx, const std:
     args.insert(args.end(), argv.begin(), argv.end());
     args.push_back("''");
 
-    // Convert to vector of char *
-    std::vector<const char *> c_args;
-    transform(
-            args.begin(),
-            args.end(),
-            std::back_inserter(c_args),
-            [](const std::string& s) { return s.c_str(); } );
-    std::vector<size_t> lengths;
-    transform(
-            args.begin(),
-            args.end(),
-            std::back_inserter(lengths),
-            [](const std::string &s) { return s.length(); } );
-
     RedisCommand command;
-    command.formatArgv(static_cast<int>(c_args.size()), c_args.data(), lengths.data());
+    command.format(args);
 
     std::set<std::string> ret;
     try

diff --git a/common/rediscommand.cpp b/common/rediscommand.cpp
@@ -7,47 +7,36 @@ using namespace std;
 namespace swss {
 
 RedisCommand::RedisCommand()
- : temp(NULL),
-   len(0)
 {
 }
 
 RedisCommand::~RedisCommand()
 {
-    redisFreeCommand(temp);
 }
 
 void RedisCommand::format(const char *fmt, ...)
 {
-    if (temp != nullptr)
-    {
-        redisFreeCommand(temp);
-        temp = nullptr;
-    }
-
+    char *temp = nullptr;
     va_list ap;
     va_start(ap, fmt);
-    len = redisvFormatCommand(&temp, fmt, ap);
+    int len = redisvFormatCommand(&temp, fmt, ap);
     va_end(ap);
     if (len == -1) {
         throw std::bad_alloc();
     } else if (len == -2) {
         throw std::invalid_argument("fmt");
     }
+    m_str.reset(temp, len);
 }
 
 void RedisCommand::formatArgv(int argc, const char **argv, const size_t *argvlen)
 {
-    if (temp != nullptr)
-    {
-        redisFreeCommand(temp);
-        temp = nullptr;
-    }
-
-    len = redisFormatCommandArgv(&temp, argc, argv, argvlen);
+    char *temp = nullptr;
+    int len = redisFormatCommandArgv(&temp, argc, argv, argvlen);
     if (len == -1) {
         throw std::bad_alloc();
     }
+    m_str.reset(temp, len);
 }
 
 void RedisCommand::format(const vector<string> &commands)
@@ -99,14 +88,9 @@ void RedisCommand::formatHDEL(const std::string& key, const std::vector<std::str
 {
     if (fields.empty()) throw std::invalid_argument("empty values");
 
-    std::vector<const char *> args = {"HDEL", key.c_str()};
-    std::vector<size_t> lens = {4, key.size()};
-    for (const std::string &f : fields)
-    {
-        args.push_back(f.c_str());
-        lens.push_back(f.size());
-    }
-    formatArgv(static_cast<int>(args.size()), args.data(), lens.data());
+    std::vector<string> args = {"HDEL", key};
+    args.insert(args.end(), fields.begin(), fields.end());
+    format(args);
 }
 
 /* Format EXPIRE key field command */
@@ -129,14 +113,12 @@ void RedisCommand::formatDEL(const std::string& key)
 
 const char *RedisCommand::c_str() const
 {
-    return temp;
+    return m_str.getStr();
 }
 
 size_t RedisCommand::length() const
 {
-    if (len <= 0)
-        return 0;
-    return static_cast<size_t>(len);
+    return static_cast<size_t>(m_str.getLen());
 }
 
 }
diff --git a/common/rediscommand.h b/common/rediscommand.h
@@ -7,6 +7,8 @@
 #include <stdexcept>
 #include <map>
 
+#include "redisstring.h"
+
 namespace swss {
 
 typedef std::pair<std::string, std::string> FieldValueTuple;
@@ -69,8 +71,7 @@ class RedisCommand {
     size_t length() const;
 
 private:
-    char *temp;
-    int len;
+    RedisString m_str;
 };
 
 template<typename InputIterator>
@@ -81,15 +82,15 @@ void RedisCommand::formatHSET(const std::string &key,
 
     const char* cmd = "HSET";
 
-    std::vector<const char*> args = { cmd, key.c_str() };
+    std::vector<std::string> args = { cmd, key.c_str() };
 
     for (auto i = start; i != stop; i++)
     {
-        args.push_back(fvField(*i).c_str());
-        args.push_back(fvValue(*i).c_str());
+        args.push_back(fvField(*i));
+        args.push_back(fvValue(*i));
     }
 
-    formatArgv((int)args.size(), args.data(), NULL);
+    format(args);
 }
 
 }
diff --git a/common/redisstring.cpp b/common/redisstring.cpp
@@ -0,0 +1,37 @@
+#include "redisstring.h"
+#include <hiredis/hiredis.h>
+
+namespace swss {
+
+RedisString::RedisString()
+    : m_data(nullptr),
+      m_len(0)
+{
+}
+
+RedisString::~RedisString()
+{
+    reset();
+}
+
+void RedisString::reset(char *str, int len)
+{
+    if (m_data != nullptr)
+    {
+        redisFreeCommand(m_data);
+    }
+    m_data = str;
+    m_len = len;
+}
+
+const char *RedisString::getStr() const
+{
+    return m_data;
+}
+
+int RedisString::getLen() const
+{
+    return m_len;
+}
+
+}
diff --git a/common/redisstring.h b/common/redisstring.h
@@ -0,0 +1,23 @@
+#pragma once
+
+namespace swss {
+
+class RedisString {
+public:
+
+    RedisString();
+
+    ~RedisString();
+
+    void reset(char *str = nullptr, int len = 0);
+
+    const char *getStr() const;
+
+    int getLen() const;
+
+private:
+    char *m_data;
+    int m_len;
+};
+
+}