Skip to content

Commit 24c9c19

Browse files
committed
pcn-firewall: small code cleanup
Signed-off-by: Matteo Bertrone <m.bertrone@gmail.com>
1 parent f71203a commit 24c9c19

7 files changed

Lines changed: 17 additions & 25 deletions

File tree

src/services/pcn-firewall/src/Chain.cpp

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ ChainResetCountersOutputJsonObject Chain::resetCounters() {
179179
}
180180

181181
dynamic_cast<Firewall::DefaultAction *>(
182-
programs->at(ModulesConstants::DEFAULTACTION))->flushCounters(name);
182+
programs->at(ModulesConstants::DEFAULTACTION))->flushCounters();
183183

184184
counters_.clear();
185185

@@ -285,8 +285,8 @@ void Chain::updateChain() {
285285
// SWAP indexes
286286
*horus_swap_ = !(*horus_swap_);
287287

288-
uint8_t horus_index_new = -1;
289-
uint8_t horus_index_old = -1;
288+
uint8_t horus_index_new;
289+
uint8_t horus_index_old;
290290

291291
// Apply Horus optimization
292292

@@ -523,7 +523,6 @@ void Chain::updateChain() {
523523
if (index == startingIndex) {
524524
firstProgramLoaded = actionlookup;
525525
}
526-
++index;
527526

528527
for (auto &rule : rules_) {
529528
actionlookup->updateTableValue(rule->getId(),
@@ -643,8 +642,7 @@ std::vector<std::shared_ptr<ChainRule>> Chain::getRuleList() {
643642
defaultRule.setDescription("Default Policy");
644643
defaultRule.setId(rules_.size());
645644

646-
rules.push_back(
647-
std::shared_ptr<ChainRule>(new ChainRule(*this, defaultRule)));
645+
rules.push_back(std::make_shared<ChainRule>(*this, defaultRule));
648646

649647
return rules;
650648
}
@@ -663,7 +661,7 @@ void Chain::addRule(const uint32_t &id, const ChainRuleJsonObject &conf) {
663661
if (newRule == nullptr) {
664662
// Totally useless, but it is needed to avoid the compiler making wrong
665663
// assumptions and reordering
666-
throw new std::runtime_error("I won't be thrown");
664+
throw std::runtime_error("I won't be thrown");
667665

668666
} else if (rules_.size() <= id && newRule != nullptr) {
669667
rules_.resize(rules_.size() + 1);
@@ -775,7 +773,7 @@ ChainInsertOutputJsonObject Chain::insert(ChainInsertInputJsonObject input) {
775773
if (newRule == nullptr) {
776774
// Totally useless, but it is needed to avoid the compiler making wrong
777775
// assumptions and reordering
778-
throw new std::runtime_error("I won't be thrown");
776+
throw std::runtime_error("I won't be thrown");
779777

780778
} else if (rules_.size() >= id && newRule != nullptr) {
781779
rules_.resize(rules_.size() + 1);

src/services/pcn-firewall/src/Chain.h

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -97,9 +97,6 @@ class Chain : public ChainBase {
9797

9898
void updateChain();
9999

100-
/*Like getRuleList, but without the last (fake) default rule*/
101-
std::vector<std::shared_ptr<ChainRule>> getRealRuleList();
102-
103100
static bool ipFromRulesToMap(
104101
const uint8_t &type, std::map<struct IpAddr, std::vector<uint64_t>> &ips,
105102
const std::vector<std::shared_ptr<ChainRule>> &rules);

src/services/pcn-firewall/src/ChainStats.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -166,8 +166,8 @@ std::shared_ptr<ChainStats> ChainStats::getDefaultActionCounters(
166166
auto actionProgram = dynamic_cast<Firewall::DefaultAction *>(
167167
programs->at(ModulesConstants::DEFAULTACTION));
168168

169-
csj.setPkts(actionProgram->getPktsCount(parent.name));
170-
csj.setBytes(actionProgram->getBytesCount(parent.name));
169+
csj.setPkts(actionProgram->getPktsCount());
170+
csj.setBytes(actionProgram->getBytesCount());
171171

172172
return std::make_shared<ChainStats>(parent, csj);
173173
}

src/services/pcn-firewall/src/Firewall.h

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -177,14 +177,13 @@ class Firewall : public FirewallBase {
177177

178178
class Horus : public Program {
179179
public:
180-
enum HorusType { HORUS_INGRESS, HORUS_EGRESS };
181180

182181
Horus(const int &index, Firewall &outer, const ChainNameEnum &direction,
183182
const std::map<struct HorusRule, struct HorusValue> &horus);
184183
~Horus();
185184

186185
std::string getCode();
187-
std::string defaultActionString(ChainNameEnum chain); // Overrides
186+
std::string defaultActionString();
188187

189188
uint64_t getPktsCount(int rule_number);
190189
uint64_t getBytesCount(int rule_number);
@@ -195,7 +194,6 @@ class Firewall : public FirewallBase {
195194
void updateMap(const std::map<struct HorusRule, struct HorusValue> &horus);
196195

197196
private:
198-
HorusType type_;
199197
std::map<struct HorusRule, struct HorusValue> horus_;
200198
};
201199

@@ -305,9 +303,9 @@ class Firewall : public FirewallBase {
305303
DefaultAction(const int &index, const ChainNameEnum &direction,
306304
Firewall &outer);
307305
~DefaultAction();
308-
uint64_t getPktsCount(ChainNameEnum chain);
309-
uint64_t getBytesCount(ChainNameEnum chain);
310-
void flushCounters(ChainNameEnum chain);
306+
uint64_t getPktsCount();
307+
uint64_t getBytesCount();
308+
void flushCounters();
311309
std::string getCode();
312310
};
313311

src/services/pcn-firewall/src/Utils.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -551,7 +551,6 @@ void Chain::horusFromRulesToMap(
551551
struct HorusRule key;
552552
struct HorusValue value;
553553

554-
// bool first_rule = true;
555554
uint64_t set_fields = 0;
556555

557556
// find match pattern for first rule (e.g. 01101 means ips proto ports set)
@@ -587,7 +586,7 @@ bool Chain::conntrackFromRulesToMap(
587586

588587
for (auto const &rule : rules) {
589588
try {
590-
rule_state = ChainRuleConntrackEnum_to_int(rule->getConntrack());
589+
ChainRuleConntrackEnum_to_int(rule->getConntrack());
591590
conntrackRulePresent = true;
592591
} catch (...) {
593592
}

src/services/pcn-firewall/src/modules/DefaultAction.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ std::string Firewall::DefaultAction::getCode() {
4646
return noMacroCode;
4747
}
4848

49-
uint64_t Firewall::DefaultAction::getPktsCount(ChainNameEnum chain) {
49+
uint64_t Firewall::DefaultAction::getPktsCount() {
5050
std::string tableName = "pktsCounter";
5151

5252
try {
@@ -61,7 +61,7 @@ uint64_t Firewall::DefaultAction::getPktsCount(ChainNameEnum chain) {
6161
}
6262
}
6363

64-
uint64_t Firewall::DefaultAction::getBytesCount(ChainNameEnum chain) {
64+
uint64_t Firewall::DefaultAction::getBytesCount() {
6565
std::string tableName = "bytesCounter";
6666

6767
try {
@@ -77,7 +77,7 @@ uint64_t Firewall::DefaultAction::getBytesCount(ChainNameEnum chain) {
7777
}
7878
}
7979

80-
void Firewall::DefaultAction::flushCounters(ChainNameEnum chain) {
80+
void Firewall::DefaultAction::flushCounters() {
8181
std::string pktsTableName = "pktsCounter";
8282
std::string bytesTableName = "bytesCounter";
8383

src/services/pcn-firewall/src/modules/Horus.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ Firewall::Horus::Horus(
6969

7070
Firewall::Horus::~Horus() {}
7171

72-
std::string Firewall::Horus::defaultActionString(ChainNameEnum chain) {
72+
std::string Firewall::Horus::defaultActionString() {
7373
return "";
7474
}
7575

0 commit comments

Comments
 (0)