On Wed, 2010-07-14 at 16:29 +0200, Nikola Pajkovsky wrote:
list values shoud be parsed as uniquie
CCpp = Bugzilla, Bugzilla must be reduce to Bugzilla
Signed-off-by: Nikola Pajkovsky <npajkovs(a)redhat.com>
---
src/Daemon/Settings.cpp | 70 +++++++++++++++++++++++++++++++++++++++++-----
1 files changed, 62 insertions(+), 8 deletions(-)
diff --git a/src/Daemon/Settings.cpp b/src/Daemon/Settings.cpp
index eb12286..b2fbb47 100644
--- a/src/Daemon/Settings.cpp
+++ b/src/Daemon/Settings.cpp
@@ -74,6 +74,20 @@ map_cron_t g_settings_mapCron;
* Loading
*/
+static int exist_item(const vector_pair_string_string_t args, const std::string key,
const std::string value)
Passing vector_pair_string_string_t _by value_ is very expensive.
Change it to reference: const vector_pair_string_string_t &args.
Similarly, "const std::string" should probably also be
"const std::string &"...
+{
+ for (size_t ii = 0; ii < args.size(); ++ii)
+ {
+ if (args[ii] == make_pair(key, value))
... because you may write it so that key and value are used just once,
thus no need to have value parameters:
pair_string_string_t pair = make_pair(key, value); <==HERE
for (...)
{
if (args[ii] == pair) ....
+ {
+ VERB3 log(" Skiping existed pair(%s,%s)", key.c_str(),
value.c_str());
" Skipping existing pair...."
+ return 1;
+ }
+ }
+
+ return 0;
+}
+
static set_string_t ParseList(const char* pList)
{
unsigned ii;
@@ -83,7 +97,11 @@ static set_string_t ParseList(const char* pList)
{
if (pList[ii] == ',')
{
- set.insert(item);
+ set_string_t::const_iterator end = set.end();
+ set_string_t::const_iterator it = set.find(item);
+ if (end == it)
+ set.insert(item);
+
Looks ok.
item = "";
}
else
@@ -93,11 +111,43 @@ static set_string_t ParseList(const char* pList)
}
if (item != "")
{
- set.insert(item);
+ set_string_t::const_iterator end = set.end();
+ set_string_t::const_iterator it = set.find(item);
+ if (end == it)
+ set.insert(item);
Looks ok.
}
return set;
}
+// Need to be rewritten, it's just a workaround
+static std::string parse_list_unique(const char* list)
+{
+ std::string item;
+ std::string ret;
+ for (unsigned int ii = 0; list[ii]; ii++)
+ {
+ if (list[ii] == ',')
+ {
+ if (strstr(ret.c_str(), item.c_str()) == NULL)
+ ret = ret + item + ",";
+ item = "";
+ }
+ else
+ item += list[ii];
+ }
+
+ if (item != "")
+ {
+ if (strstr(ret.c_str(), item.c_str()) == NULL)
+ ret = ret + item;
+ }
+ size_t len = strlen(ret.c_str());
+ if (ret[--len] == ',')
+ ret[len] = '\0'; <=== ??
This is not ok. string container might get very confused by this.
+
+ return ret;
+}
In general entire function is, indeed, a hack.
It allows "Logger,Log,Lo,Bugzilla"...
I hesitate to ACK it for the next build.
/* Format: name, name(param),name("param with spaces \"and
quotes\"") */
static vector_pair_string_string_t ParseListWithArgs(const char *pValue, int *err)
{
@@ -145,7 +195,7 @@ static vector_pair_string_string_t ParseListWithArgs(const char
*pValue, int *er
}
if (pValue[ii] == ')')
{
- if (is_arg)
+ if (is_arg && !exist_item(pluginsWithArgs, action, item))
{
VERB3 log(" adding (%s,%s)", action.c_str(), item.c_str());
pluginsWithArgs.push_back(make_pair(action, item));
@@ -163,12 +213,15 @@ static vector_pair_string_string_t ParseListWithArgs(const char
*pValue, int *er
}
if (pValue[ii] == ',' && !is_arg)
{
- if (item != "")
+ if (item != "" && !exist_item(pluginsWithArgs, item,
""))
{
VERB3 log(" adding (%s,%s)", item.c_str(), "");
pluginsWithArgs.push_back(make_pair(item, ""));
item = "";
}
+ else
+ item = "";
+
Looks like you can just move item = "" out of if() body:
if() { ... }
item = "";
continue;
}
item += pValue[ii];
@@ -185,7 +238,7 @@ static vector_pair_string_string_t ParseListWithArgs(const char
*pValue, int *er
*err = 1;
error_msg("Parser error: Unclosed argument in \"%s\"",
pValue);
}
- else if (item != "")
+ else if (item != "" && !exist_item(pluginsWithArgs, item,
""))
{
VERB3 log(" adding (%s,%s)", item.c_str(), "");
pluginsWithArgs.push_back(make_pair(item, ""));
@@ -455,19 +508,20 @@ static int ReadConfigurationFromFile(FILE *fp)
{
if (s_mapSectionCommon[key] != "")
s_mapSectionCommon[key] += ",";
- s_mapSectionCommon[key] += value;
+ s_mapSectionCommon[key] += parse_list_unique(value.c_str());
}
else if (section == SECTION_ANALYZER_ACTIONS_AND_REPORTERS)
{
if (s_mapSectionAnalyzerActionsAndReporters[key] != "")
s_mapSectionAnalyzerActionsAndReporters[key] += ",";
- s_mapSectionAnalyzerActionsAndReporters[key] += value;
+
+ s_mapSectionAnalyzerActionsAndReporters[key] +=
parse_list_unique(value.c_str());
Since parse_list_unique() is not pretty, how about not doing the above
change (and thus throwing away parse_list_unique) and just eliminating
duplicates later, in ParseAnalyzerActionsAndReporters(),
where commas are parsed already?
}
else if (section == SECTION_CRON)
{
if (s_mapSectionCron[key] != "")
s_mapSectionCron[key] += ",";
- s_mapSectionCron[key] += value;
+ s_mapSectionCron[key] += parse_list_unique(value.c_str());
}
else
{
--
vda