You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by "brbzull0 (via GitHub)" <gi...@apache.org> on 2023/09/20 10:44:47 UTC
[GitHub] [trafficserver] brbzull0 opened a new pull request, #10475: CID-1508880: Re-work function to avoid dead-code.
brbzull0 opened a new pull request, #10475:
URL: https://github.com/apache/trafficserver/pull/10475
closes https://github.com/apache/trafficserver/issues/10428
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] brbzull0 commented on a diff in pull request #10475: CID-1508880: Re-work function to avoid dead-code.
Posted by "brbzull0 (via GitHub)" <gi...@apache.org>.
brbzull0 commented on code in PR #10475:
URL: https://github.com/apache/trafficserver/pull/10475#discussion_r1331434507
##########
include/shared/rpc/RPCClient.h:
##########
@@ -64,23 +64,22 @@ class RPCClient
_client.disconnect();
return {bw.data(), bw.size()};
}
- case IPCSocketClient::ReadStatus::BUFFER_FULL: {
- throw std::runtime_error(
- swoc::bwprint(text, "Buffer full, not enough space to read the response. Buffer size: {}", BUFFER_SIZE));
- } break;
+ case IPCSocketClient::ReadStatus::BUFFER_FULL:
+ swoc::bwprint(err_text, "Buffer full, not enough space to read the response. Buffer size: {}", BUFFER_SIZE);
+ break;
default:
- throw std::runtime_error("Something happened, we can't read the response");
+ err_text = "Something happened, we can't read the response";
break;
}
} else {
- throw std::runtime_error(swoc::bwprint(text, "Node seems not available: {}", std ::strerror(errno)));
+ swoc::bwprint(err_text, "Node seems not available: {}", std ::strerror(errno));
}
} catch (std::exception const &ex) {
- _client.disconnect();
- throw std::runtime_error(swoc::bwprint(text, "RPC Node Error: {}", ex.what()));
+ swoc::bwprint(err_text, "RPC Node Error: {}", ex.what());
}
-
- return {};
+ _client.disconnect();
+ // If we've got this far, then there must be an error to report back.
+ throw std::runtime_error(err_text);
Review Comment:
not sure if coverity will complain about this as there is no return :P
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] moonchen commented on pull request #10475: CID-1508880: Re-work function to avoid dead-code.
Posted by "moonchen (via GitHub)" <gi...@apache.org>.
moonchen commented on PR #10475:
URL: https://github.com/apache/trafficserver/pull/10475#issuecomment-1736150017
The change looks correct. I'm curious: do any compilers complain if all you do is remove that last return statement?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] brbzull0 merged pull request #10475: CID-1508880: Re-work function to avoid dead-code.
Posted by "brbzull0 (via GitHub)" <gi...@apache.org>.
brbzull0 merged PR #10475:
URL: https://github.com/apache/trafficserver/pull/10475
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] brbzull0 commented on a diff in pull request #10475: CID-1508880: Re-work function to avoid dead-code.
Posted by "brbzull0 (via GitHub)" <gi...@apache.org>.
brbzull0 commented on code in PR #10475:
URL: https://github.com/apache/trafficserver/pull/10475#discussion_r1331434507
##########
include/shared/rpc/RPCClient.h:
##########
@@ -64,23 +64,22 @@ class RPCClient
_client.disconnect();
return {bw.data(), bw.size()};
}
- case IPCSocketClient::ReadStatus::BUFFER_FULL: {
- throw std::runtime_error(
- swoc::bwprint(text, "Buffer full, not enough space to read the response. Buffer size: {}", BUFFER_SIZE));
- } break;
+ case IPCSocketClient::ReadStatus::BUFFER_FULL:
+ swoc::bwprint(err_text, "Buffer full, not enough space to read the response. Buffer size: {}", BUFFER_SIZE);
+ break;
default:
- throw std::runtime_error("Something happened, we can't read the response");
+ err_text = "Something happened, we can't read the response";
break;
}
} else {
- throw std::runtime_error(swoc::bwprint(text, "Node seems not available: {}", std ::strerror(errno)));
+ swoc::bwprint(err_text, "Node seems not available: {}", std ::strerror(errno));
}
} catch (std::exception const &ex) {
- _client.disconnect();
- throw std::runtime_error(swoc::bwprint(text, "RPC Node Error: {}", ex.what()));
+ swoc::bwprint(err_text, "RPC Node Error: {}", ex.what());
}
-
- return {};
+ _client.disconnect();
+ // If we've got this far, then there must be an error to report back.
+ throw std::runtime_error(err_text);
Review Comment:
not sure if coverity will complain about this as there is no return :P but we will see.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] brbzull0 commented on pull request #10475: CID-1508880: Re-work function to avoid dead-code.
Posted by "brbzull0 (via GitHub)" <gi...@apache.org>.
brbzull0 commented on PR #10475:
URL: https://github.com/apache/trafficserver/pull/10475#issuecomment-1736360397
> The change looks correct. I'm curious: do any compilers complain if all you do is remove that last return statem
more likely they will complain, I've seen it. But not sure if modern compilers do it.
In any case I think this looks better than before.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org