[Xorp-hackers] [PATCH 2/8] xorp: rtrmgr: Fix using of return varibles in xrl
igorm at etf.rs
igorm at etf.rs
Tue Mar 20 10:02:41 PDT 2012
From: Igor Maravic <igorm at etf.rs>
Without this patch, when we read and write from/to same xrl variable,
value that should be committed to variable is overwritten with fake value.
For example:
...
parent @: uint {
%create: xrl "$(prot.targetname)/prot/0.1/create?node:u32=$(@)";
%activate: xrl "$(prot.targetname)/prot/0.1/activate?node:u32=$(@)->child_ret:u32=$(@.child)";
%update: xrl "$(prot.targetname)/prot/0.1/update?node:u32=$(@)->child_ret:u32=$(@.child)";
child {
%set: xrl "$(prot.targetname)/prot/0.1/set?node:u32=$(parent.@)&child:u32=$(@)";
}
}
...
In this case in function set we would get bogus value for variable child, because we are writing to it
from update functions.
This patch fixes that.
Case when we are calling %create + %set + %activate is working without this patch.
Signed-off-by: Igor Maravic <igorm at etf.rs>
---
xorp/libxipc/xrl_atom.cc | 11 +++-
xorp/libxipc/xrl_atom.hh | 108 ++++++++++++++++++++------------------
xorp/rtrmgr/conf_tree_node.cc | 10 ++++
xorp/rtrmgr/conf_tree_node.hh | 9 +++
xorp/rtrmgr/template_commands.cc | 5 ++
xorp/rtrmgr/xorp_client.cc | 28 +++++-----
6 files changed, 102 insertions(+), 69 deletions(-)
diff --git a/xorp/libxipc/xrl_atom.cc b/xorp/libxipc/xrl_atom.cc
index a31d54a..e8a892b 100644
--- a/xorp/libxipc/xrl_atom.cc
+++ b/xorp/libxipc/xrl_atom.cc
@@ -385,6 +385,7 @@ XrlAtom::copy(const XrlAtom& xa)
_type = xa._type;
_have_data = xa._have_data;
+ _has_fake_args = xa._has_fake_args;
_own = true;
if (_have_data) {
@@ -483,6 +484,7 @@ XrlAtom::discard_dynamic()
// ... Your type should free allocated memory here ...
}
_have_data = false;
+ _has_fake_args = false;
}
}
@@ -505,7 +507,8 @@ XrlAtom::str() const
XrlAtom::XrlAtom(const char* serialized) throw (InvalidString, BadName)
: _type(xrlatom_no_type),
_have_data(false),
- _own(true)
+ _own(true),
+ _has_fake_args(false)
{
const char *start, *sep;
@@ -547,7 +550,8 @@ XrlAtom::XrlAtom(const string& name, XrlAtomType t,
const string& serialized_data) throw (InvalidString)
: _type(t),
_have_data(false),
- _own(true)
+ _own(true),
+ _has_fake_args(false)
{
set_name(name);
ssize_t bad_pos = data_from_c_str(serialized_data.c_str());
@@ -559,7 +563,8 @@ XrlAtom::XrlAtom(const char* name, XrlAtomType t,
const string& serialized_data) throw (InvalidString)
: _type(t),
_have_data(false),
- _own(true)
+ _own(true),
+ _has_fake_args(false)
{
set_name(name);
ssize_t bad_pos = data_from_c_str(serialized_data.c_str());
diff --git a/xorp/libxipc/xrl_atom.hh b/xorp/libxipc/xrl_atom.hh
index ba996d6..7dc7a80 100644
--- a/xorp/libxipc/xrl_atom.hh
+++ b/xorp/libxipc/xrl_atom.hh
@@ -119,20 +119,20 @@ public:
string _name;
};
- XrlAtom() : _type(xrlatom_no_type), _have_data(false), _own(true) {}
+ XrlAtom() : _type(xrlatom_no_type), _have_data(false), _own(true), _has_fake_args(false) {}
~XrlAtom();
// type but no data constructors
XrlAtom(XrlAtomType t)
- : _type(t), _have_data(false), _own(true) {}
+ : _type(t), _have_data(false), _own(true), _has_fake_args(false) {}
XrlAtom(const string& name, XrlAtomType t) throw (BadName)
- : _type(t), _have_data(false), _own(true) {
+ : _type(t), _have_data(false), _own(true), _has_fake_args(false) {
set_name(name);
}
XrlAtom(const char* name, XrlAtomType t) throw (BadName)
- : _type(t), _have_data(false), _own(true) {
+ : _type(t), _have_data(false), _own(true), _has_fake_args(false) {
set_name(name);
}
@@ -153,81 +153,81 @@ public:
// int32 constructors
explicit XrlAtom(const int32_t& value)
- : _type(xrlatom_int32), _have_data(true), _own(true), _i32val(value) {}
+ : _type(xrlatom_int32), _have_data(true), _own(true), _has_fake_args(false), _i32val(value) {}
- XrlAtom(const char* name, int32_t value) throw (BadName)
- : _type(xrlatom_int32), _have_data(true), _own(true) ,_i32val(value) {
+ XrlAtom(const char* name, int32_t value, bool fake_args = false) throw (BadName)
+ : _type(xrlatom_int32), _have_data(true), _own(true), _has_fake_args(fake_args),_i32val(value) {
set_name(name);
}
// bool constructors
explicit XrlAtom(const bool& value)
: _type(xrlatom_boolean), _have_data(true),
- _own(true), _boolean(value) {}
+ _own(true), _has_fake_args(false), _boolean(value) {}
- XrlAtom(const char* name, bool value) throw (BadName)
+ XrlAtom(const char* name, bool value, bool fake_args = false) throw (BadName)
: _type(xrlatom_boolean), _have_data(true),
- _own(true), _boolean(value) {
+ _own(true), _has_fake_args(fake_args), _boolean(value) {
set_name(name);
}
// uint32 constructors
explicit XrlAtom(const uint32_t& value)
- : _type(xrlatom_uint32), _have_data(true), _own(true), _u32val(value) {}
+ : _type(xrlatom_uint32), _have_data(true), _own(true), _has_fake_args(false), _u32val(value) {}
- XrlAtom(const char* name, uint32_t value) throw (BadName)
- : _type(xrlatom_uint32), _have_data(true), _own(true), _u32val(value) {
+ XrlAtom(const char* name, uint32_t value, bool fake_args = false) throw (BadName)
+ : _type(xrlatom_uint32), _have_data(true), _own(true), _has_fake_args(fake_args), _u32val(value) {
set_name(name);
}
// ipv4 constructors
explicit XrlAtom(const IPv4& addr)
- : _type(xrlatom_ipv4), _have_data(true), _own(true),
+ : _type(xrlatom_ipv4), _have_data(true), _own(true), _has_fake_args(false),
_ipv4(addr) {}
- XrlAtom(const char* name, const IPv4& addr) throw (BadName)
- : _type(xrlatom_ipv4), _have_data(true), _own(true),
+ XrlAtom(const char* name, const IPv4& addr, bool fake_args = false) throw (BadName)
+ : _type(xrlatom_ipv4), _have_data(true), _own(true), _has_fake_args(fake_args),
_ipv4(addr) {
set_name(name);
}
// ipv4net constructors
explicit XrlAtom(const IPv4Net& subnet)
- : _type(xrlatom_ipv4net), _have_data(true), _own(true),
+ : _type(xrlatom_ipv4net), _have_data(true), _own(true), _has_fake_args(false),
_ipv4net(subnet) {}
- XrlAtom(const char* name, const IPv4Net& subnet) throw (BadName)
- : _type(xrlatom_ipv4net), _have_data(true), _own(true),
+ XrlAtom(const char* name, const IPv4Net& subnet, bool fake_args = false) throw (BadName)
+ : _type(xrlatom_ipv4net), _have_data(true), _own(true), _has_fake_args(fake_args),
_ipv4net(subnet) {
set_name(name);
}
// ipv6 constructors
explicit XrlAtom(const IPv6& addr)
- : _type(xrlatom_ipv6), _have_data(true), _own(true),
+ : _type(xrlatom_ipv6), _have_data(true), _own(true), _has_fake_args(false),
_ipv6(new IPv6(addr)) {}
XrlAtom(const char* name, const IPv6& addr) throw (BadName)
- : _type(xrlatom_ipv6), _have_data(true), _own(true),
+ : _type(xrlatom_ipv6), _have_data(true), _own(true), _has_fake_args(false),
_ipv6(new IPv6(addr)) {
set_name(name);
}
// ipv6 net constructors
explicit XrlAtom(const IPv6Net& subnet)
- : _type(xrlatom_ipv6net), _have_data(true), _own(true),
+ : _type(xrlatom_ipv6net), _have_data(true), _own(true), _has_fake_args(false),
_ipv6net(new IPv6Net(subnet)) {}
- XrlAtom(const char* name, const IPv6Net& subnet) throw (BadName)
- : _type(xrlatom_ipv6net), _have_data(true), _own(true),
+ XrlAtom(const char* name, const IPv6Net& subnet, bool fake_args = false) throw (BadName)
+ : _type(xrlatom_ipv6net), _have_data(true), _own(true), _has_fake_args(fake_args),
_ipv6net(new IPv6Net(subnet)) {
set_name(name);
}
// IPvX constructors - there is no underlying IPvX type
// data is cast to IPv4 or IPv6.
- XrlAtom(const char* name, const IPvX& ipvx) throw (BadName)
- : _have_data(true), _own(true)
+ XrlAtom(const char* name, const IPvX& ipvx, bool fake_args = false) throw (BadName)
+ : _have_data(true), _own(true), _has_fake_args(fake_args)
{
set_name(name);
if (ipvx.is_ipv4()) {
@@ -243,8 +243,8 @@ public:
// IPvXNet constructors - there is no underlying IPvXNet type
// data is cast to IPv4Net or IPv6Net.
- XrlAtom(const char* name, const IPvXNet& ipvxnet) throw (BadName)
- : _have_data(true), _own(true)
+ XrlAtom(const char* name, const IPvXNet& ipvxnet, bool fake_args = false) throw (BadName)
+ : _have_data(true), _own(true), _has_fake_args(fake_args)
{
set_name(name);
if (ipvxnet.is_ipv4()) {
@@ -260,83 +260,83 @@ public:
// mac constructors
explicit XrlAtom(const Mac& mac)
- : _type(xrlatom_mac), _have_data(true), _own(true),
+ : _type(xrlatom_mac), _have_data(true), _own(true), _has_fake_args(false),
_mac(new Mac(mac)) {}
- XrlAtom(const char* name, const Mac& mac) throw (BadName)
- : _type(xrlatom_mac), _have_data(true), _own(true),
+ XrlAtom(const char* name, const Mac& mac, bool fake_args = false) throw (BadName)
+ : _type(xrlatom_mac), _have_data(true), _own(true), _has_fake_args(fake_args),
_mac(new Mac(mac)) {
set_name(name);
}
// text constructors
explicit XrlAtom(const string& txt)
- : _type(xrlatom_text), _have_data(true), _own(true),
+ : _type(xrlatom_text), _have_data(true), _own(true), _has_fake_args(false),
_text(new string(txt)) {}
- XrlAtom(const char* name, const string& txt) throw (BadName)
- : _type(xrlatom_text), _have_data(true), _own(true),
+ XrlAtom(const char* name, const string& txt, bool fake_args = false) throw (BadName)
+ : _type(xrlatom_text), _have_data(true), _own(true), _has_fake_args(fake_args),
_text(new string(txt)) {
set_name(name);
}
// list constructors
explicit XrlAtom(const XrlAtomList& l)
- : _type(xrlatom_list), _have_data(true), _own(true),
+ : _type(xrlatom_list), _have_data(true), _own(true), _has_fake_args(false),
_list(new XrlAtomList(l)) {}
- XrlAtom(const char* name, const XrlAtomList& l) throw (BadName)
- : _type(xrlatom_list), _have_data(true), _own(true),
+ XrlAtom(const char* name, const XrlAtomList& l, bool fake_args = false) throw (BadName)
+ : _type(xrlatom_list), _have_data(true), _own(true), _has_fake_args(fake_args),
_list(new XrlAtomList(l)) {
set_name(name);
}
// binary
XrlAtom(const char* name, const vector<uint8_t>& data)
- : _type(xrlatom_binary), _have_data(true), _own(true),
+ : _type(xrlatom_binary), _have_data(true), _own(true), _has_fake_args(false),
_binary(new vector<uint8_t>(data)) {
set_name(name);
}
- XrlAtom(const char* name, const uint8_t* data, size_t data_bytes)
- : _type(xrlatom_binary), _have_data(true), _own(true),
+ XrlAtom(const char* name, const uint8_t* data, size_t data_bytes, bool fake_args = false)
+ : _type(xrlatom_binary), _have_data(true), _own(true), _has_fake_args(fake_args),
_binary(new vector<uint8_t>(data, data + data_bytes)) {
set_name(name);
}
XrlAtom(const vector<uint8_t>& data)
- : _type(xrlatom_binary), _have_data(true), _own(true),
+ : _type(xrlatom_binary), _have_data(true), _own(true), _has_fake_args(false),
_binary(new vector<uint8_t>(data)) {}
XrlAtom(const uint8_t* data, size_t data_bytes)
- : _type(xrlatom_binary), _have_data(true), _own(true),
+ : _type(xrlatom_binary), _have_data(true), _own(true), _has_fake_args(false),
_binary(new vector<uint8_t>(data, data + data_bytes)) {}
// int64 constructors
explicit XrlAtom(const int64_t& value)
- : _type(xrlatom_int64), _have_data(true), _own(true), _i64val(value) {}
+ : _type(xrlatom_int64), _have_data(true), _own(true), _has_fake_args(false), _i64val(value) {}
- XrlAtom(const char* name, int64_t value) throw (BadName)
- : _type(xrlatom_int64), _have_data(true), _own(true), _i64val(value) {
+ XrlAtom(const char* name, int64_t value, bool fake_args = false) throw (BadName)
+ : _type(xrlatom_int64), _have_data(true), _own(true), _has_fake_args(fake_args), _i64val(value) {
set_name(name);
}
// uint64 constructors
explicit XrlAtom(const uint64_t& value)
- : _type(xrlatom_uint64), _have_data(true), _own(true), _u64val(value) {}
+ : _type(xrlatom_uint64), _have_data(true), _own(true), _has_fake_args(false), _u64val(value) {}
- XrlAtom(const char* name, uint64_t value) throw (BadName)
- : _type(xrlatom_uint64), _have_data(true), _own(true), _u64val(value) {
+ XrlAtom(const char* name, uint64_t value, bool fake_args = false) throw (BadName)
+ : _type(xrlatom_uint64), _have_data(true), _own(true), _has_fake_args(fake_args), _u64val(value) {
set_name(name);
}
// fp64 constructors
explicit XrlAtom(const fp64_t& value)
- : _type(xrlatom_fp64), _have_data(true), _own(true), _fp64val(value) {}
+ : _type(xrlatom_fp64), _have_data(true), _own(true), _has_fake_args(false), _fp64val(value) {}
- XrlAtom(const char* name, fp64_t value) throw (BadName)
- : _type(xrlatom_fp64), _have_data(true), _own(true), _fp64val(value) {
+ XrlAtom(const char* name, fp64_t value, bool fake_args = false) throw (BadName)
+ : _type(xrlatom_fp64), _have_data(true), _own(true), _has_fake_args(fake_args), _fp64val(value) {
set_name(name);
}
@@ -345,7 +345,7 @@ public:
// Copy operations
XrlAtom(const XrlAtom& x)
- : _type(xrlatom_no_type), _have_data(false), _own(true) {
+ : _type(xrlatom_no_type), _have_data(false), _own(true), _has_fake_args(false) {
copy(x);
}
XrlAtom& operator=(const XrlAtom& x) {
@@ -362,8 +362,11 @@ public:
const string value() const;
const bool& has_data() const { return _have_data; }
+ const bool& has_fake_args() const { return _has_fake_args; }
const XrlAtomType& type() const { return _type; }
+ void using_real_args() { _has_fake_args = false; }
+
// The following accessors may throw accessor exceptions...
const bool& boolean() const throw (NoData, WrongType);
const int32_t& int32() const throw (NoData, WrongType);
@@ -482,6 +485,7 @@ private:
bool _have_data;
string _atom_name;
bool _own;
+ bool _has_fake_args;
union {
bool _boolean;
diff --git a/xorp/rtrmgr/conf_tree_node.cc b/xorp/rtrmgr/conf_tree_node.cc
index 12adc59..bb8fd25 100644
--- a/xorp/rtrmgr/conf_tree_node.cc
+++ b/xorp/rtrmgr/conf_tree_node.cc
@@ -1866,6 +1866,16 @@ ConfigTreeNode::expand_varname_to_matchlist(const vector<string>& parts,
}
}
+void
+ConfigTreeNode::value_to_node_existing_value(const string& varname, string& value)
+{
+ VarType type;
+ ConfigTreeNode* node = find_varname_node(varname, type);
+ if (node && node->has_value())
+ value = node->value();
+}
+
+
bool
ConfigTreeNode::set_variable(const string& varname, const string& value)
{
diff --git a/xorp/rtrmgr/conf_tree_node.hh b/xorp/rtrmgr/conf_tree_node.hh
index 0101a00..4bff458 100644
--- a/xorp/rtrmgr/conf_tree_node.hh
+++ b/xorp/rtrmgr/conf_tree_node.hh
@@ -174,6 +174,15 @@ public:
bool has_undeleted_children() const;
virtual void update_node_id_position();
+ /**
+ * This function sets variable value, from it's arguments,
+ * to existing value of node.
+ *
+ * If there is no node defined with varname or node
+ * doesn't have value, variable value stays unchanged.
+ */
+ void value_to_node_existing_value(const string& varname, string& value);
+
protected:
bool split_up_varname(const string& varname,
list<string>& var_parts) const;
diff --git a/xorp/rtrmgr/template_commands.cc b/xorp/rtrmgr/template_commands.cc
index 8164d72..00422bd 100644
--- a/xorp/rtrmgr/template_commands.cc
+++ b/xorp/rtrmgr/template_commands.cc
@@ -1419,6 +1419,11 @@ Command::process_xrl_action_return_arguments(XrlArgs* xrl_args,
string value = returned_atom.value();
debug_msg("found atom = %s\n", returned_atom.str().c_str());
debug_msg("found value = %s\n", value.c_str());
+
+ if (returned_atom.has_fake_args()) {
+ ctn->value_to_node_existing_value(varname, value);
+ returned_atom.using_real_args();
+ }
ctn->set_variable(varname, value);
}
diff --git a/xorp/rtrmgr/xorp_client.cc b/xorp/rtrmgr/xorp_client.cc
index 437fcb6..f88b3de 100644
--- a/xorp/rtrmgr/xorp_client.cc
+++ b/xorp/rtrmgr/xorp_client.cc
@@ -126,46 +126,46 @@ XorpClient::fake_return_args(const string& xrl_return_spec)
case xrlatom_no_type:
XLOG_UNREACHABLE();
case xrlatom_int32:
- xargs.add(XrlAtom(atom.name().c_str(), (int32_t)0) );
+ xargs.add(XrlAtom(atom.name().c_str(), (int32_t)0, true) );
break;
case xrlatom_uint32:
- xargs.add(XrlAtom(atom.name().c_str(), (uint32_t)0) );
+ xargs.add(XrlAtom(atom.name().c_str(), (uint32_t)0, true) );
break;
case xrlatom_ipv4:
- xargs.add(XrlAtom(atom.name().c_str(), IPv4("0.0.0.0")) );
+ xargs.add(XrlAtom(atom.name().c_str(), IPv4("0.0.0.0"), true) );
break;
case xrlatom_ipv4net:
- xargs.add(XrlAtom(atom.name().c_str(), IPv4Net("0.0.0.0/0")) );
+ xargs.add(XrlAtom(atom.name().c_str(), IPv4Net("0.0.0.0/0"), true) );
break;
case xrlatom_text:
- xargs.add(XrlAtom(atom.name().c_str(), string("")) );
+ xargs.add(XrlAtom(atom.name().c_str(), string(""), true) );
break;
case xrlatom_ipv6:
- xargs.add(XrlAtom(atom.name().c_str(), IPv6("::")) );
+ xargs.add(XrlAtom(atom.name().c_str(), IPv6("::"), true) );
break;
case xrlatom_ipv6net:
- xargs.add(XrlAtom(atom.name().c_str(), IPv6Net("::/0")) );
+ xargs.add(XrlAtom(atom.name().c_str(), IPv6Net("::/0"), true) );
break;
case xrlatom_mac:
- xargs.add(XrlAtom(atom.name().c_str(), Mac("00:00:00:00:00:00")) );
+ xargs.add(XrlAtom(atom.name().c_str(), Mac("00:00:00:00:00:00"), true) );
break;
case xrlatom_list:
- xargs.add(XrlAtom(atom.name().c_str(), XrlAtomList()) );
+ xargs.add(XrlAtom(atom.name().c_str(), XrlAtomList(), true) );
break;
case xrlatom_boolean:
- xargs.add(XrlAtom(atom.name().c_str(), false) );
+ xargs.add(XrlAtom(atom.name().c_str(), false, true) );
break;
case xrlatom_binary:
- xargs.add(XrlAtom(atom.name().c_str(), data, sizeof(data)) );
+ xargs.add(XrlAtom(atom.name().c_str(), data, sizeof(data), true) );
break;
case xrlatom_uint64:
- xargs.add(XrlAtom(atom.name().c_str(), (uint64_t)0) );
+ xargs.add(XrlAtom(atom.name().c_str(), (uint64_t)0, true) );
break;
case xrlatom_int64:
- xargs.add(XrlAtom(atom.name().c_str(), (int64_t)0) );
+ xargs.add(XrlAtom(atom.name().c_str(), (int64_t)0, true) );
break;
case xrlatom_fp64:
- xargs.add(XrlAtom(atom.name().c_str(), (fp64_t)0) );
+ xargs.add(XrlAtom(atom.name().c_str(), (fp64_t)0, true) );
break;
}
}
--
1.7.5.4
More information about the Xorp-hackers
mailing list