From 42434a0384cf113cbb721867f05659422dbae7bc Mon Sep 17 00:00:00 2001 From: Rudigus Date: Sun, 12 Apr 2020 02:22:45 -0300 Subject: [PATCH] Improve error explanations related to UPNP and UPNPDevice --- modules/upnp/upnp.cpp | 7 +++---- modules/upnp/upnp_device.cpp | 22 +++++++++++----------- 2 files changed, 14 insertions(+), 15 deletions(-) diff --git a/modules/upnp/upnp.cpp b/modules/upnp/upnp.cpp index 856f4188aaa..988bf3017d9 100644 --- a/modules/upnp/upnp.cpp +++ b/modules/upnp/upnp.cpp @@ -44,9 +44,8 @@ bool UPNP::is_common_device(const String &dev) const { } int UPNP::discover(int timeout, int ttl, const String &device_filter) { - ERR_FAIL_COND_V(timeout < 0, UPNP_RESULT_INVALID_PARAM); - ERR_FAIL_COND_V(ttl < 0, UPNP_RESULT_INVALID_PARAM); - ERR_FAIL_COND_V(ttl > 255, UPNP_RESULT_INVALID_PARAM); + ERR_FAIL_COND_V_MSG(timeout < 0, UPNP_RESULT_INVALID_PARAM, "The response's wait time can't be negative."); + ERR_FAIL_COND_V_MSG(ttl < 0 || ttl > 255, UPNP_RESULT_INVALID_PARAM, "The time-to-live must be set between 0 and 255 (inclusive)."); devices.clear(); @@ -264,7 +263,7 @@ void UPNP::clear_devices() { } Ref UPNP::get_gateway() const { - ERR_FAIL_COND_V(devices.size() < 1, nullptr); + ERR_FAIL_COND_V_MSG(devices.size() < 1, nullptr, "Couldn't find any UPNPDevices."); for (int i = 0; i < devices.size(); i++) { Ref dev = get_device(i); diff --git a/modules/upnp/upnp_device.cpp b/modules/upnp/upnp_device.cpp index c21826d14d4..40eb6106a0d 100644 --- a/modules/upnp/upnp_device.cpp +++ b/modules/upnp/upnp_device.cpp @@ -35,7 +35,7 @@ #include String UPNPDevice::query_external_address() const { - ERR_FAIL_COND_V(!is_valid_gateway(), ""); + ERR_FAIL_COND_V_MSG(!is_valid_gateway(), "", "The Internet Gateway Device must be valid."); char addr[16]; int i = UPNP_GetExternalIPAddress( @@ -43,17 +43,17 @@ String UPNPDevice::query_external_address() const { igd_service_type.utf8().get_data(), (char *)&addr); - ERR_FAIL_COND_V(i != UPNPCOMMAND_SUCCESS, ""); + ERR_FAIL_COND_V_MSG(i != UPNPCOMMAND_SUCCESS, "", "Couldn't get external IP address."); return String(addr); } int UPNPDevice::add_port_mapping(int port, int port_internal, String desc, String proto, int duration) const { - ERR_FAIL_COND_V(!is_valid_gateway(), UPNP::UPNP_RESULT_INVALID_GATEWAY); - ERR_FAIL_COND_V(port < 1 || port > 65535, UPNP::UPNP_RESULT_INVALID_PORT); - ERR_FAIL_COND_V(port_internal < 0 || port_internal > 65535, UPNP::UPNP_RESULT_INVALID_PORT); // Needs to allow 0 because 0 signifies "use external port as internal port" - ERR_FAIL_COND_V(proto != "UDP" && proto != "TCP", UPNP::UPNP_RESULT_INVALID_PROTOCOL); - ERR_FAIL_COND_V(duration < 0, UPNP::UPNP_RESULT_INVALID_DURATION); + ERR_FAIL_COND_V_MSG(!is_valid_gateway(), UPNP::UPNP_RESULT_INVALID_GATEWAY, "The Internet Gateway Device must be valid."); + ERR_FAIL_COND_V_MSG(port < 1 || port > 65535, UPNP::UPNP_RESULT_INVALID_PORT, "The port number must be set between 1 and 65535 (inclusive)."); + ERR_FAIL_COND_V_MSG(port_internal < 0 || port_internal > 65535, UPNP::UPNP_RESULT_INVALID_PORT, "The port number must be set between 0 and 65535 (inclusive)."); // Needs to allow 0 because 0 signifies "use external port as internal port" + ERR_FAIL_COND_V_MSG(proto != "UDP" && proto != "TCP", UPNP::UPNP_RESULT_INVALID_PROTOCOL, "The protocol must be either TCP or UDP."); + ERR_FAIL_COND_V_MSG(duration < 0, UPNP::UPNP_RESULT_INVALID_DURATION, "The port mapping's lease duration can't be negative."); if (port_internal < 1) { port_internal = port; @@ -70,14 +70,14 @@ int UPNPDevice::add_port_mapping(int port, int port_internal, String desc, Strin nullptr, // Remote host, always nullptr as IGDs don't support it duration > 0 ? itos(duration).utf8().get_data() : nullptr); - ERR_FAIL_COND_V(i != UPNPCOMMAND_SUCCESS, UPNP::upnp_result(i)); + ERR_FAIL_COND_V_MSG(i != UPNPCOMMAND_SUCCESS, UPNP::upnp_result(i), "Couldn't add port mapping."); return UPNP::UPNP_RESULT_SUCCESS; } int UPNPDevice::delete_port_mapping(int port, String proto) const { - ERR_FAIL_COND_V(port < 1 || port > 65535, UPNP::UPNP_RESULT_INVALID_PORT); - ERR_FAIL_COND_V(proto != "UDP" && proto != "TCP", UPNP::UPNP_RESULT_INVALID_PROTOCOL); + ERR_FAIL_COND_V_MSG(port < 1 || port > 65535, UPNP::UPNP_RESULT_INVALID_PORT, "The port number must be set between 1 and 65535 (inclusive)."); + ERR_FAIL_COND_V_MSG(proto != "UDP" && proto != "TCP", UPNP::UPNP_RESULT_INVALID_PROTOCOL, "The protocol must be either TCP or UDP."); int i = UPNP_DeletePortMapping( igd_control_url.utf8().get_data(), @@ -87,7 +87,7 @@ int UPNPDevice::delete_port_mapping(int port, String proto) const { nullptr // Remote host, always nullptr as IGDs don't support it ); - ERR_FAIL_COND_V(i != UPNPCOMMAND_SUCCESS, UPNP::upnp_result(i)); + ERR_FAIL_COND_V_MSG(i != UPNPCOMMAND_SUCCESS, UPNP::upnp_result(i), "Couldn't delete port mapping."); return UPNP::UPNP_RESULT_SUCCESS; }