From 29e7674e56bf2b829bba22def2760d034a76e788 Mon Sep 17 00:00:00 2001 From: Sadie Powell Date: Mon, 11 Mar 2024 13:53:05 +0000 Subject: Replace convertTo/stringify with non-throwing alternatives. Having these throw is terrible for ergonomics and there are loads of places where the exception was either silently ignored or not handled at all. Having a function which returns an optional and another that returns a default works a lot better imo. --- modules/database/db_atheme.cpp | 27 ++++++++-------------- modules/database/db_flatfile.cpp | 9 ++------ modules/database/db_old.cpp | 4 ++-- modules/database/db_redis.cpp | 50 ++++++++++++++++------------------------ modules/database/db_sql.cpp | 18 ++++++--------- modules/database/db_sql_live.cpp | 15 ++++++------ 6 files changed, 47 insertions(+), 76 deletions(-) (limited to 'modules/database') diff --git a/modules/database/db_atheme.cpp b/modules/database/db_atheme.cpp index 3e613562b..3f62c29ca 100644 --- a/modules/database/db_atheme.cpp +++ b/modules/database/db_atheme.cpp @@ -56,16 +56,7 @@ public: template std::enable_if_t, Numeric> GetNum() { - try - { - auto token = Get(); - std::stringstream stream(token.str()); - Numeric ntoken = 0; - stream >> ntoken; - return ntoken; - } - catch (const ConvertException &) { } - return 0; + return Anope::Convert(Get(), 0); } // Retrieves the entire row. @@ -869,7 +860,7 @@ private: else if (key == "private:close:reason") data->suspend_reason = value; else if (key == "private:close:timestamp") - data->suspend_ts = convertTo(value); + data->suspend_ts = Anope::Convert(value, 0); else if (key == "private:entrymsg") { auto *eml = ci->Require("entrymsg"); @@ -891,19 +882,19 @@ private: else if (key == "private:klinechan:reason") data->suspend_reason = value; else if (key == "private:klinechan:timestamp") - data->suspend_ts = convertTo(value); + data->suspend_ts = Anope::Convert(value, 0); else if (key == "private:mark:reason") data->info_message = value; else if (key == "private:mark:setter") data->info_adder = value; else if (key == "private:mark:timestamp") - data->info_ts = convertTo(value); + data->info_ts = Anope::Convert(value, 0); else if (key == "private:topic:setter") ci->last_topic_setter = value; else if (key == "private:topic:text") ci->last_topic = value; else if (key == "private:topic:ts") - ci->last_topic_time = convertTo(value); + ci->last_topic_time = Anope::Convert(value, 0); else Log(this) << "Unknown channel metadata " << key << " = " << value; @@ -953,7 +944,7 @@ private: auto kill = Config->GetModule("nickserv")->Get("kill", "60s"); auto killquick = Config->GetModule("nickserv")->Get("killquick", "20s"); - auto secs = convertTo(value); + auto secs = Anope::Convert(value, kill); if (secs >= kill) nc->Extend("KILLPROTECT"); else if (secs >= killquick) @@ -966,7 +957,7 @@ private: else if (key == "private:freeze:reason") data->suspend_reason = value; else if (key == "private:freeze:timestamp") - data->suspend_ts = convertTo(value); + data->suspend_ts = Anope::Convert(value, 0); else if (key == "private:host:actual") data->last_real_mask = value; else if (key == "private:host:vhost") @@ -978,13 +969,13 @@ private: else if (key == "private:mark:setter") data->info_adder = value; else if (key == "private:mark:timestamp") - data->info_ts = convertTo(value); + data->info_ts = Anope::Convert(value, 0); else if (key == "private:usercloak") data->vhost = value; else if (key == "private:usercloak-assigner") data->vhost_creator = value; else if (key == "private:usercloak-timestamp") - data->vhost_ts = convertTo(value); + data->vhost_ts = Anope::Convert(value, 0); else if (key.compare(0, 18, "private:usercloak:", 18) == 0) data->vhost_nick[key.substr(18)] = value; else diff --git a/modules/database/db_flatfile.cpp b/modules/database/db_flatfile.cpp index d2225ce88..63c18f744 100644 --- a/modules/database/db_flatfile.cpp +++ b/modules/database/db_flatfile.cpp @@ -52,12 +52,7 @@ public: { if (token.find("ID ") == 0) { - try - { - this->id = convertTo(token.substr(3)); - } - catch (const ConvertException &) { } - + this->id = Anope::Convert(token.substr(3), 0); continue; } else if (token.find("DATA ") != 0) @@ -136,7 +131,7 @@ class DBFlatFile final for (const auto &db : dbs) { const Anope::string &oldname = Anope::DataDir + "/" + db; - Anope::string newname = Anope::DataDir + "/backups/" + db + "-" + stringify(tm->tm_year + 1900) + Anope::printf("-%02i-", tm->tm_mon + 1) + Anope::printf("%02i", tm->tm_mday); + Anope::string newname = Anope::DataDir + "/backups/" + db + "-" + Anope::ToString(tm->tm_year + 1900) + Anope::printf("-%02i-", tm->tm_mon + 1) + Anope::printf("%02i", tm->tm_mday); /* Backup already exists or no database to backup */ if (Anope::IsFile(newname) || !Anope::IsFile(oldname)) diff --git a/modules/database/db_old.cpp b/modules/database/db_old.cpp index c3e82ea39..e95cc19ef 100644 --- a/modules/database/db_old.cpp +++ b/modules/database/db_old.cpp @@ -152,7 +152,7 @@ static void process_mlock(ChannelInfo *ci, uint32_t lock, bool status, uint32_t if (cm && ml) { if (limit && mlock_info.c == 'l') - ml->SetMLock(cm, status, stringify(*limit)); + ml->SetMLock(cm, status, Anope::ToString(*limit)); else if (key && mlock_info.c == 'k') ml->SetMLock(cm, status, *key); else @@ -889,7 +889,7 @@ static void LoadChannels() } } else - access->AccessUnserialize(stringify(level)); + access->AccessUnserialize(Anope::ToString(level)); } Anope::string mask; diff --git a/modules/database/db_redis.cpp b/modules/database/db_redis.cpp index b4abcab7f..413729887 100644 --- a/modules/database/db_redis.cpp +++ b/modules/database/db_redis.cpp @@ -162,7 +162,7 @@ public: std::vector args; args.emplace_back("HGETALL"); - args.push_back("hash:" + t->GetName() + ":" + stringify(obj->id)); + args.push_back("hash:" + t->GetName() + ":" + Anope::ToString(obj->id)); /* Get object attrs to clear before updating */ redis->SendCommand(new Updater(this, t->GetName(), obj->id), args); @@ -248,7 +248,7 @@ public: std::vector args; args.emplace_back("HGETALL"); - args.push_back("hash:" + t->GetName() + ":" + stringify(obj->id)); + args.push_back("hash:" + t->GetName() + ":" + Anope::ToString(obj->id)); /* Get all of the attributes for this object */ redis->SendCommand(new Deleter(this, t->GetName(), obj->id), args); @@ -278,19 +278,14 @@ void TypeLoader::OnResult(const Reply &r) if (reply->type != Reply::BULK) continue; - int64_t id; - try - { - id = convertTo(reply->bulk); - } - catch (const ConvertException &) - { + auto i = Anope::TryConvert(reply->bulk); + if (!i) continue; - } + auto id = i.value(); std::vector args; args.emplace_back("HGETALL"); - args.push_back("hash:" + this->type + ":" + stringify(id)); + args.push_back("hash:" + this->type + ":" + Anope::ToString(id)); me->redis->SendCommand(new ObjectLoader(me, this->type, id), args); } @@ -364,7 +359,7 @@ void Deleter::OnResult(const Reply &r) std::vector args; args.emplace_back("DEL"); - args.push_back("hash:" + this->type + ":" + stringify(this->id)); + args.push_back("hash:" + this->type + ":" + Anope::ToString(this->id)); /* Delete hash object */ me->redis->SendCommand(NULL, args); @@ -372,7 +367,7 @@ void Deleter::OnResult(const Reply &r) args.clear(); args.emplace_back("SREM"); args.push_back("ids:" + this->type); - args.push_back(stringify(this->id)); + args.push_back(Anope::ToString(this->id)); /* Delete id from ids set */ me->redis->SendCommand(NULL, args); @@ -385,7 +380,7 @@ void Deleter::OnResult(const Reply &r) args.clear(); args.emplace_back("SREM"); args.push_back("value:" + this->type + ":" + key->bulk + ":" + value->bulk); - args.push_back(stringify(this->id)); + args.push_back(Anope::ToString(this->id)); /* Delete value -> object id */ me->redis->SendCommand(NULL, args); @@ -428,7 +423,7 @@ void Updater::OnResult(const Reply &r) std::vector args; args.emplace_back("SREM"); args.push_back("value:" + this->type + ":" + key->bulk + ":" + value->bulk); - args.push_back(stringify(this->id)); + args.push_back(Anope::ToString(this->id)); /* Delete value -> object id */ me->redis->SendCommand(NULL, args); @@ -438,12 +433,12 @@ void Updater::OnResult(const Reply &r) std::vector args; args.emplace_back("SADD"); args.push_back("ids:" + this->type); - args.push_back(stringify(obj->id)); + args.push_back(Anope::ToString(obj->id)); me->redis->SendCommand(NULL, args); args.clear(); args.emplace_back("HMSET"); - args.push_back("hash:" + this->type + ":" + stringify(obj->id)); + args.push_back("hash:" + this->type + ":" + Anope::ToString(obj->id)); for (const auto &[key, value] : data.data) { @@ -454,7 +449,7 @@ void Updater::OnResult(const Reply &r) args2.emplace_back("SADD"); args2.push_back("value:" + this->type + ":" + key + ":" + value->str()); - args2.push_back(stringify(obj->id)); + args2.push_back(Anope::ToString(obj->id)); /* Add to value -> object id set */ me->redis->SendCommand(NULL, args2); @@ -505,16 +500,11 @@ void SubscriptionListener::OnResult(const Reply &r) if (s_type == NULL) return; - uint64_t obj_id; - try - { - obj_id = convertTo(id); - } - catch (const ConvertException &) - { + auto oid = Anope::TryConvert(id); + if (!oid.has_value()) return; - } + auto obj_id = oid.value(); if (op == "hset" || op == "hdel") { Serializable *s = s_type->objects[obj_id]; @@ -564,7 +554,7 @@ void SubscriptionListener::OnResult(const Reply &r) std::vector args; args.emplace_back("SREM"); args.push_back("ids:" + type); - args.push_back(stringify(s->id)); + args.push_back(Anope::ToString(s->id)); /* Delete object from id set */ me->redis->SendCommand(NULL, args); @@ -604,7 +594,7 @@ void ModifiedObject::OnResult(const Reply &r) std::vector args; args.emplace_back("SREM"); args.push_back("value:" + st->GetName() + ":" + key + ":" + value->str()); - args.push_back(stringify(this->id)); + args.push_back(Anope::ToString(this->id)); /* Delete value -> object id */ me->redis->SendCommand(NULL, args); @@ -633,7 +623,7 @@ void ModifiedObject::OnResult(const Reply &r) std::vector args; args.emplace_back("SADD"); args.push_back("value:" + st->GetName() + ":" + key + ":" + value->str()); - args.push_back(stringify(obj->id)); + args.push_back(Anope::ToString(obj->id)); /* Add to value -> object id set */ me->redis->SendCommand(NULL, args); @@ -642,7 +632,7 @@ void ModifiedObject::OnResult(const Reply &r) std::vector args; args.emplace_back("SADD"); args.push_back("ids:" + st->GetName()); - args.push_back(stringify(obj->id)); + args.push_back(Anope::ToString(obj->id)); /* Add to type -> id set */ me->redis->SendCommand(NULL, args); diff --git a/modules/database/db_sql.cpp b/modules/database/db_sql.cpp index 24b5c1f1f..d8fe89972 100644 --- a/modules/database/db_sql.cpp +++ b/modules/database/db_sql.cpp @@ -209,7 +209,7 @@ public: return; Serialize::Type *s_type = obj->GetSerializableType(); if (s_type && obj->id > 0) - this->RunBackground("DELETE FROM `" + this->prefix + s_type->GetName() + "` WHERE `id` = " + stringify(obj->id)); + this->RunBackground("DELETE FROM `" + this->prefix + s_type->GetName() + "` WHERE `id` = " + Anope::ToString(obj->id)); this->updated_items.erase(obj); } @@ -240,18 +240,14 @@ public: data[key] << value; Serializable *obj = sb->Unserialize(NULL, data); - try - { - if (obj) - obj->id = convertTo(res.Get(j, "id")); - } - catch (const ConvertException &) - { - Log(this) << "Unable to convert id for object #" << j << " of type " << sb->GetName(); - } - if (obj) { + auto oid = Anope::TryConvert(res.Get(j, "id")); + if (oid.has_value()) + obj->id = oid.value(); + else + Log(this) << "Unable to convert id for object #" << j << " of type " << sb->GetName(); + /* The Unserialize operation is destructive so rebuild the data for UpdateCache. * Also the old data may contain columns that we don't use, so we reserialize the * object to know for sure our cache is consistent diff --git a/modules/database/db_sql_live.cpp b/modules/database/db_sql_live.cpp index d97fe6683..839126d41 100644 --- a/modules/database/db_sql_live.cpp +++ b/modules/database/db_sql_live.cpp @@ -163,7 +163,7 @@ public: if (s_type) { if (obj->id > 0) - this->RunQuery("DELETE FROM `" + this->prefix + s_type->GetName() + "` WHERE `id` = " + stringify(obj->id)); + this->RunQuery("DELETE FROM `" + this->prefix + s_type->GetName() + "` WHERE `id` = " + Anope::ToString(obj->id)); s_type->objects.erase(obj->id); } this->updated_items.erase(obj); @@ -185,17 +185,16 @@ public: { const std::map &row = res.Row(i); - unsigned int id; - try - { - id = convertTo(res.Get(i, "id")); - } - catch (const ConvertException &) + + + auto oid = Anope::TryConvert(res.Get(i, "id")); + if (!oid.has_value()) { Log(LOG_DEBUG) << "Unable to convert id from " << obj->GetName(); continue; } + auto id = oid.value(); if (res.Get(i, "timestamp").empty()) { clear_null = true; @@ -237,7 +236,7 @@ public: else { if (!s) - this->RunQuery("UPDATE `" + prefix + obj->GetName() + "` SET `timestamp` = " + this->SQL->FromUnixtime(obj->GetTimestamp()) + " WHERE `id` = " + stringify(id)); + this->RunQuery("UPDATE `" + prefix + obj->GetName() + "` SET `timestamp` = " + this->SQL->FromUnixtime(obj->GetTimestamp()) + " WHERE `id` = " + Anope::ToString(id)); else delete s; } -- cgit