Skip to content

Commit

Permalink
Merge bitcoin#25736: univalue: Remove unused and confusing set*() ret…
Browse files Browse the repository at this point in the history 
…urn value

fa7bef2 univalue: Remove unused and confusing set*() return value (MacroFake)

Pull request description:

  The value is:

  * currently unused, and useless without `[[nodiscard]]`
  * confusing, because it is always `true`, unless a num-string is set

  Instead of adding `[[nodiscard]]`, throw when setting is not possible.

ACKs for top commit:
  shaavan:
    ACK fa7bef2
  aureleoules:
    ACK fa7bef2.

Tree-SHA512: 0d74f96f34cb93b66019ab75e12334c964630cc83434f22e58cc7a4fff2ee96a5767e42ab37f08acb67aeacba6811b09c75f1edc68d5e903ccfc59b1c82de891
  • Loading branch information
MacroFake committed Aug 2, 2022
2 parents ce3b756 + fa7bef2 commit 816ca01
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 55 deletions.
4 changes: 2 additions & 2 deletions src/test/rpc_tests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -181,10 +181,10 @@ BOOST_AUTO_TEST_CASE(rpc_format_monetary_values)
BOOST_CHECK_EQUAL(ValueFromAmount(std::numeric_limits<CAmount>::min()).write(), "-92233720368.54775808");
}

static UniValue ValueFromString(const std::string &str)
static UniValue ValueFromString(const std::string& str) noexcept
{
UniValue value;
BOOST_CHECK(value.setNumStr(str));
value.setNumStr(str);
return value;
}

Expand Down
20 changes: 10 additions & 10 deletions src/univalue/include/univalue.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,16 +47,16 @@ class UniValue {

void clear();

bool setNull();
bool setBool(bool val);
bool setNumStr(const std::string& val);
bool setInt(uint64_t val);
bool setInt(int64_t val);
bool setInt(int val_) { return setInt((int64_t)val_); }
bool setFloat(double val);
bool setStr(const std::string& val);
bool setArray();
bool setObject();
void setNull();
void setBool(bool val);
void setNumStr(const std::string& val);
void setInt(uint64_t val);
void setInt(int64_t val);
void setInt(int val_) { return setInt(int64_t{val_}); }
void setFloat(double val);
void setStr(const std::string& val);
void setArray();
void setObject();

enum VType getType() const { return typ; }
const std::string& getValStr() const { return val; }
Expand Down
33 changes: 13 additions & 20 deletions src/univalue/lib/univalue.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,19 +23,17 @@ void UniValue::clear()
values.clear();
}

bool UniValue::setNull()
void UniValue::setNull()
{
clear();
return true;
}

bool UniValue::setBool(bool val_)
void UniValue::setBool(bool val_)
{
clear();
typ = VBOOL;
if (val_)
val = "1";
return true;
}

static bool validNumStr(const std::string& s)
Expand All @@ -46,18 +44,18 @@ static bool validNumStr(const std::string& s)
return (tt == JTOK_NUMBER);
}

bool UniValue::setNumStr(const std::string& val_)
void UniValue::setNumStr(const std::string& val_)
{
if (!validNumStr(val_))
return false;
if (!validNumStr(val_)) {
throw std::runtime_error{"The string '" + val_ + "' is not a valid JSON number"};
}

clear();
typ = VNUM;
val = val_;
return true;
}

bool UniValue::setInt(uint64_t val_)
void UniValue::setInt(uint64_t val_)
{
std::ostringstream oss;

Expand All @@ -66,7 +64,7 @@ bool UniValue::setInt(uint64_t val_)
return setNumStr(oss.str());
}

bool UniValue::setInt(int64_t val_)
void UniValue::setInt(int64_t val_)
{
std::ostringstream oss;

Expand All @@ -75,37 +73,32 @@ bool UniValue::setInt(int64_t val_)
return setNumStr(oss.str());
}

bool UniValue::setFloat(double val_)
void UniValue::setFloat(double val_)
{
std::ostringstream oss;

oss << std::setprecision(16) << val_;

bool ret = setNumStr(oss.str());
typ = VNUM;
return ret;
return setNumStr(oss.str());
}

bool UniValue::setStr(const std::string& val_)
void UniValue::setStr(const std::string& val_)
{
clear();
typ = VSTR;
val = val_;
return true;
}

bool UniValue::setArray()
void UniValue::setArray()
{
clear();
typ = VARR;
return true;
}

bool UniValue::setObject()
void UniValue::setObject()
{
clear();
typ = VOBJ;
return true;
}

void UniValue::push_back(const UniValue& val_)
Expand Down
45 changes: 22 additions & 23 deletions src/univalue/test/object.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ void univalue_constructor()
BOOST_CHECK_EQUAL(v3.getValStr(), "foo");

UniValue numTest;
BOOST_CHECK(numTest.setNumStr("82"));
numTest.setNumStr("82");
BOOST_CHECK(numTest.isNum());
BOOST_CHECK_EQUAL(numTest.getValStr(), "82");

Expand Down Expand Up @@ -93,33 +93,33 @@ void univalue_push_throw()
void univalue_typecheck()
{
UniValue v1;
BOOST_CHECK(v1.setNumStr("1"));
v1.setNumStr("1");
BOOST_CHECK(v1.isNum());
BOOST_CHECK_THROW(v1.get_bool(), std::runtime_error);

{
UniValue v_negative;
BOOST_CHECK(v_negative.setNumStr("-1"));
v_negative.setNumStr("-1");
BOOST_CHECK_THROW(v_negative.getInt<uint8_t>(), std::runtime_error);
BOOST_CHECK_EQUAL(v_negative.getInt<int8_t>(), -1);
}

UniValue v2;
BOOST_CHECK(v2.setBool(true));
v2.setBool(true);
BOOST_CHECK_EQUAL(v2.get_bool(), true);
BOOST_CHECK_THROW(v2.getInt<int>(), std::runtime_error);

UniValue v3;
BOOST_CHECK(v3.setNumStr("32482348723847471234"));
v3.setNumStr("32482348723847471234");
BOOST_CHECK_THROW(v3.getInt<int64_t>(), std::runtime_error);
BOOST_CHECK(v3.setNumStr("1000"));
v3.setNumStr("1000");
BOOST_CHECK_EQUAL(v3.getInt<int64_t>(), 1000);

UniValue v4;
BOOST_CHECK(v4.setNumStr("2147483648"));
v4.setNumStr("2147483648");
BOOST_CHECK_EQUAL(v4.getInt<int64_t>(), 2147483648);
BOOST_CHECK_THROW(v4.getInt<int>(), std::runtime_error);
BOOST_CHECK(v4.setNumStr("1000"));
v4.setNumStr("1000");
BOOST_CHECK_EQUAL(v4.getInt<int>(), 1000);
BOOST_CHECK_THROW(v4.get_str(), std::runtime_error);
BOOST_CHECK_EQUAL(v4.get_real(), 1000);
Expand All @@ -146,55 +146,55 @@ void univalue_set()
BOOST_CHECK(v.isNull());
BOOST_CHECK_EQUAL(v.getValStr(), "");

BOOST_CHECK(v.setObject());
v.setObject();
BOOST_CHECK(v.isObject());
BOOST_CHECK_EQUAL(v.size(), 0);
BOOST_CHECK_EQUAL(v.getType(), UniValue::VOBJ);
BOOST_CHECK(v.empty());

BOOST_CHECK(v.setArray());
v.setArray();
BOOST_CHECK(v.isArray());
BOOST_CHECK_EQUAL(v.size(), 0);

BOOST_CHECK(v.setStr("zum"));
v.setStr("zum");
BOOST_CHECK(v.isStr());
BOOST_CHECK_EQUAL(v.getValStr(), "zum");

BOOST_CHECK(v.setFloat(-1.01));
v.setFloat(-1.01);
BOOST_CHECK(v.isNum());
BOOST_CHECK_EQUAL(v.getValStr(), "-1.01");

BOOST_CHECK(v.setInt((int)1023));
v.setInt(int{1023});
BOOST_CHECK(v.isNum());
BOOST_CHECK_EQUAL(v.getValStr(), "1023");

BOOST_CHECK(v.setInt((int64_t)-1023LL));
v.setInt(int64_t{-1023LL});
BOOST_CHECK(v.isNum());
BOOST_CHECK_EQUAL(v.getValStr(), "-1023");

BOOST_CHECK(v.setInt((uint64_t)1023ULL));
v.setInt(uint64_t{1023ULL});
BOOST_CHECK(v.isNum());
BOOST_CHECK_EQUAL(v.getValStr(), "1023");

BOOST_CHECK(v.setNumStr("-688"));
v.setNumStr("-688");
BOOST_CHECK(v.isNum());
BOOST_CHECK_EQUAL(v.getValStr(), "-688");

BOOST_CHECK(v.setBool(false));
v.setBool(false);
BOOST_CHECK_EQUAL(v.isBool(), true);
BOOST_CHECK_EQUAL(v.isTrue(), false);
BOOST_CHECK_EQUAL(v.isFalse(), true);
BOOST_CHECK_EQUAL(v.getBool(), false);

BOOST_CHECK(v.setBool(true));
v.setBool(true);
BOOST_CHECK_EQUAL(v.isBool(), true);
BOOST_CHECK_EQUAL(v.isTrue(), true);
BOOST_CHECK_EQUAL(v.isFalse(), false);
BOOST_CHECK_EQUAL(v.getBool(), true);

BOOST_CHECK(!v.setNumStr("zombocom"));
BOOST_CHECK_THROW(v.setNumStr("zombocom"), std::runtime_error);

BOOST_CHECK(v.setNull());
v.setNull();
BOOST_CHECK(v.isNull());
}

Expand Down Expand Up @@ -352,7 +352,7 @@ void univalue_object()
BOOST_CHECK_EQUAL(obj.size(), 0);
BOOST_CHECK_EQUAL(obj.getType(), UniValue::VNULL);

BOOST_CHECK_EQUAL(obj.setObject(), true);
obj.setObject();
UniValue uv;
uv.setInt(42);
obj.__pushKV("age", uv);
Expand Down Expand Up @@ -419,7 +419,7 @@ void univalue_readwrite()
BOOST_CHECK(!v.read("{} 42"));
}

int main (int argc, char *argv[])
int main(int argc, char* argv[])
{
univalue_constructor();
univalue_push_throw();
Expand All @@ -430,4 +430,3 @@ int main (int argc, char *argv[])
univalue_readwrite();
return 0;
}

0 comments on commit 816ca01

Please sign in to comment.