shill: Adding |has_a_value| to netlink attributes.
By design, netlink attributes do not have values assigned to them when they
are created. This provides a mechanism to distinguish between existing
attributes with and without values.
BUG=chromium-os:37743
TEST=unittests.
Change-Id: I00c852a98c3be5c545a0ac404af2d7554d5076cf
Reviewed-on: https://gerrit.chromium.org/gerrit/41057
Reviewed-by: mukesh agrawal <quiche@chromium.org>
Commit-Queue: Wade Guthrie <wdg@chromium.org>
Reviewed-by: Wade Guthrie <wdg@chromium.org>
Tested-by: Wade Guthrie <wdg@chromium.org>
diff --git a/attribute_list.cc b/attribute_list.cc
index e57f734..793f83e 100644
--- a/attribute_list.cc
+++ b/attribute_list.cc
@@ -50,12 +50,14 @@
for (i = attributes_.begin(); i != attributes_.end(); ++i) {
string attribute_string;
- i->second->ToString(&attribute_string);
- StringAppendF(&output, " Attr: %s(%d)=%s, Type: %s\n",
- i->second->id_string(),
- i->second->id(),
- attribute_string.c_str(),
- i->second->datatype_string());
+ if (i->second->has_a_value()) {
+ i->second->ToString(&attribute_string);
+ StringAppendF(&output, " Attr: %s(%d) Type: %s = %s\n",
+ i->second->id_string(),
+ i->second->id(),
+ i->second->datatype_string(),
+ attribute_string.c_str());
+ }
}
return output;
}
diff --git a/config80211_unittest.cc b/config80211_unittest.cc
index 3fe068e..6a81c1e 100644
--- a/config80211_unittest.cc
+++ b/config80211_unittest.cc
@@ -907,11 +907,9 @@
WeakPtr<AttributeList> nested;
EXPECT_TRUE(message->attributes().GetNestedAttributeValue(NL80211_ATTR_CQM,
&nested));
- // TODO(wdg): Enable the following when attributes get the 'is_valid'
- // property.
- //uint32_t threshold_event;
- //EXPECT_FALSE(nested->GetU32AttributeValue(
- // NL80211_ATTR_CQM_RSSI_THRESHOLD_EVENT, &threshold_event));
+ uint32_t threshold_event;
+ EXPECT_FALSE(nested->GetU32AttributeValue(
+ NL80211_ATTR_CQM_RSSI_THRESHOLD_EVENT, &threshold_event));
uint32_t pkt_loss_event;
EXPECT_TRUE(nested->GetU32AttributeValue(
NL80211_ATTR_CQM_PKT_LOSS_EVENT, &pkt_loss_event));
diff --git a/nl80211_attribute.cc b/nl80211_attribute.cc
index e108524..6ebd084 100644
--- a/nl80211_attribute.cc
+++ b/nl80211_attribute.cc
@@ -32,7 +32,7 @@
const char *id_string,
Type datatype,
const char *datatype_string)
- : id_(id), id_string_(id_string), datatype_(datatype),
+ : has_a_value_(false), id_(id), id_string_(id_string), datatype_(datatype),
datatype_string_(datatype_string) {}
// static
@@ -206,6 +206,11 @@
string Nl80211Attribute::RawToString() const {
string output = " === RAW: ";
+ if (!has_a_value_) {
+ StringAppendF(&output, "(empty)");
+ return output;
+ }
+
nlattr *data_nlattr = const_cast<nlattr *>(data());
uint16_t length = nla_len(data_nlattr);
StringAppendF(&output, "len=%u", length);
@@ -249,6 +254,11 @@
}
bool Nl80211U8Attribute::GetU8Value(uint8_t *output) const {
+ if (!has_a_value_) {
+ SLOG(WiFi, 7) << "U8 attribute " << id_string()
+ << " hasn't been set to any value.";
+ return false;
+ }
if (output) {
*output = value_;
}
@@ -257,6 +267,7 @@
bool Nl80211U8Attribute::SetU8Value(uint8_t new_value) {
value_ = new_value;
+ has_a_value_ = true;
return true;
}
@@ -296,6 +307,11 @@
}
bool Nl80211U16Attribute::GetU16Value(uint16_t *output) const {
+ if (!has_a_value_) {
+ SLOG(WiFi, 7) << "U16 attribute " << id_string()
+ << " hasn't been set to any value.";
+ return false;
+ }
if (output) {
*output = value_;
}
@@ -304,6 +320,7 @@
bool Nl80211U16Attribute::SetU16Value(uint16_t new_value) {
value_ = new_value;
+ has_a_value_ = true;
return true;
}
@@ -342,6 +359,11 @@
}
bool Nl80211U32Attribute::GetU32Value(uint32_t *output) const {
+ if (!has_a_value_) {
+ SLOG(WiFi, 7) << "U32 attribute " << id_string()
+ << " hasn't been set to any value.";
+ return false;
+ }
if (output) {
*output = value_;
}
@@ -350,6 +372,7 @@
bool Nl80211U32Attribute::SetU32Value(uint32_t new_value) {
value_ = new_value;
+ has_a_value_ = true;
return true;
}
@@ -388,6 +411,11 @@
}
bool Nl80211U64Attribute::GetU64Value(uint64_t *output) const {
+ if (!has_a_value_) {
+ SLOG(WiFi, 7) << "U64 attribute " << id_string()
+ << " hasn't been set to any value.";
+ return false;
+ }
if (output) {
*output = value_;
}
@@ -396,6 +424,7 @@
bool Nl80211U64Attribute::SetU64Value(uint64_t new_value) {
value_ = new_value;
+ has_a_value_ = true;
return true;
}
@@ -436,13 +465,15 @@
bool Nl80211FlagAttribute::GetFlagValue(bool *output) const {
if (output) {
- *output = value_;
+ // The lack of the existence of the attribute implies 'false'.
+ *output = (has_a_value_) ? value_ : false;
}
return true;
}
bool Nl80211FlagAttribute::SetFlagValue(bool new_value) {
value_ = new_value;
+ has_a_value_ = true;
return true;
}
@@ -480,6 +511,11 @@
}
bool Nl80211StringAttribute::GetStringValue(string *output) const {
+ if (!has_a_value_) {
+ SLOG(WiFi, 7) << "String attribute " << id_string()
+ << " hasn't been set to any value.";
+ return false;
+ }
if (output) {
*output = value_;
}
@@ -488,6 +524,7 @@
bool Nl80211StringAttribute::SetStringValue(const string new_value) {
value_ = new_value;
+ has_a_value_ = true;
return true;
}
@@ -519,6 +556,11 @@
Nl80211Attribute(id, id_string, kType, kMyTypeString) {}
bool Nl80211NestedAttribute::GetNestedValue(WeakPtr<AttributeList> *output) {
+ if (!has_a_value_) {
+ SLOG(WiFi, 7) << "Nested attribute " << id_string()
+ << " hasn't been set to any value.";
+ return false;
+ }
if (output) {
*output = value_.AsWeakPtr();
}
@@ -537,10 +579,19 @@
return false;
}
- return Nl80211Attribute::InitFromNlAttr(input);
+ if (!Nl80211Attribute::InitFromNlAttr(input)) {
+ return false;
+ }
+ has_a_value_ = true;
+ return true;
}
bool Nl80211RawAttribute::GetRawValue(ByteString *output) const {
+ if (!has_a_value_) {
+ SLOG(WiFi, 7) << "Raw attribute " << id_string()
+ << " hasn't been set to any value.";
+ return false;
+ }
if (output) {
*output = data_;
}
@@ -552,7 +603,11 @@
LOG(ERROR) << "Null |output| parameter";
return false;
}
- // TODO(wdg): Make sure that 'data' is valid.
+ if (!has_a_value_) {
+ SLOG(WiFi, 7) << "Raw attribute " << id_string()
+ << " hasn't been set to any value.";
+ return false;
+ }
const uint8_t *raw_data = reinterpret_cast<const uint8_t *>(data());
int total_bytes = nla_len(data());
*output = StringPrintf("%d bytes:", total_bytes);
@@ -625,6 +680,7 @@
NL80211_ATTR_CQM_PKT_LOSS_EVENT,
nla_get_u32(cqm[NL80211_ATTR_CQM_PKT_LOSS_EVENT]));
}
+ has_a_value_ = true;
return true;
}
@@ -694,6 +750,7 @@
return false;
}
// TODO(wdg): Add code, here.
+ has_a_value_ = true;
return true;
}
diff --git a/nl80211_attribute.h b/nl80211_attribute.h
index 8d99983..9b2ede8 100644
--- a/nl80211_attribute.h
+++ b/nl80211_attribute.h
@@ -106,6 +106,8 @@
// failure.
virtual ByteString Encode() const = 0;
+ bool has_a_value() const { return has_a_value_; }
+
// Note that |nla_get_*| don't change their arguments but don't declare
// themselves as 'const', either. These methods wrap the const castness.
static char *NlaGetString(const nlattr *input) {
@@ -135,6 +137,9 @@
// class.
ByteString data_;
+ // True if a value has been assigned to the attribute; false, otherwise.
+ bool has_a_value_;
+
private:
int id_; // In the non-nested case, this is really type nl80211_attrs.
const char *id_string_;
@@ -384,7 +389,7 @@
}
bool GetNestedValue(base::WeakPtr<AttributeList> *value);
bool ToString(std::string *value) const {
- return false; // TODO(wdg):
+ return false; // TODO(wdg): Actually generate a string, here.
}
virtual ByteString Encode() const {
return ByteString(); // TODO(wdg): Actually encode the attribute.
@@ -401,7 +406,7 @@
Nl80211AttributeCqm();
bool InitFromNlAttr(const nlattr *data);
bool ToString(std::string *value) const {
- return true; // TODO(wdg): Need |ToString|.
+ return false; // TODO(wdg): Need |ToString|.
}
};
@@ -412,8 +417,8 @@
explicit Nl80211AttributeStaInfo() :
Nl80211NestedAttribute(kName, kNameString) {}
bool InitFromNlAttr(const nlattr *const_data);
- bool AsString(std::string *value) const {
- return true; // TODO(wdg): Need |AsString|.
+ bool ToString(std::string *value) const {
+ return false; // TODO(wdg): Need |ToString|.
}
};
diff --git a/nl80211_message.h b/nl80211_message.h
index 70916cb..3cec60a 100644
--- a/nl80211_message.h
+++ b/nl80211_message.h
@@ -300,7 +300,7 @@
static const uint8_t kCommand;
static const char kCommandString[];
- ErrorMessage(uint32_t error);
+ explicit ErrorMessage(uint32_t error);
virtual std::string ToString() const;