You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by "SolidWallOfCode (via GitHub)" <gi...@apache.org> on 2023/06/18 03:50:34 UTC
[GitHub] [trafficserver] SolidWallOfCode opened a new pull request, #9878: libswoc: Update Errata use in cache tool.
SolidWallOfCode opened a new pull request, #9878:
URL: https://github.com/apache/trafficserver/pull/9878
(no comment)
--
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] SolidWallOfCode commented on a diff in pull request #9878: libswoc: Update Errata use in cache tool.
Posted by "SolidWallOfCode (via GitHub)" <gi...@apache.org>.
SolidWallOfCode commented on code in PR #9878:
URL: https://github.com/apache/trafficserver/pull/9878#discussion_r1265525519
##########
include/tscpp/util/ts_errata.h:
##########
@@ -40,5 +40,16 @@ static constexpr std::array<swoc::TextView, 9> Severity_Names{
{"Diag", "Debug", "Status", "Note", "Warn", "Error", "Fatal", "Alert", "Emergency"}
};
+inline std::error_code
+ec_for()
+{
+ return {errno, std::system_category()};
Review Comment:
I considered that, but in practice `errno` is system specific (e.g., the value can vary between operating systems) and therefore, IMHO, should be `system_category`. If we look at the [documentation for system_category](https://en.cppreference.com/w/cpp/error/system_category) we see
"Obtains a reference to the static error category object for errors reported by the operating system", which is precisely what `errno` is.
--
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] SolidWallOfCode merged pull request #9878: libswoc: Update Errata use in cache tool.
Posted by "SolidWallOfCode (via GitHub)" <gi...@apache.org>.
SolidWallOfCode merged PR #9878:
URL: https://github.com/apache/trafficserver/pull/9878
--
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] bneradt commented on pull request #9878: libswoc: Update Errata use in cache tool.
Posted by "bneradt (via GitHub)" <gi...@apache.org>.
bneradt commented on PR #9878:
URL: https://github.com/apache/trafficserver/pull/9878#issuecomment-1613151166
[approve ci clang-format]
--
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] SolidWallOfCode commented on a diff in pull request #9878: libswoc: Update Errata use in cache tool.
Posted by "SolidWallOfCode (via GitHub)" <gi...@apache.org>.
SolidWallOfCode commented on code in PR #9878:
URL: https://github.com/apache/trafficserver/pull/9878#discussion_r1265688598
##########
include/tscpp/util/ts_errata.h:
##########
@@ -40,5 +40,16 @@ static constexpr std::array<swoc::TextView, 9> Severity_Names{
{"Diag", "Debug", "Status", "Note", "Warn", "Error", "Fatal", "Alert", "Emergency"}
};
+inline std::error_code
+ec_for()
+{
+ return {errno, std::system_category()};
+}
+inline std::error_code
+ec_for(int err)
+{
+ return {err, std::system_category()};
+}
+
Review Comment:
Changed to `make_errno_code`.
--
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] jpeach commented on a diff in pull request #9878: libswoc: Update Errata use in cache tool.
Posted by "jpeach (via GitHub)" <gi...@apache.org>.
jpeach commented on code in PR #9878:
URL: https://github.com/apache/trafficserver/pull/9878#discussion_r1251476811
##########
include/tscpp/util/ts_errata.h:
##########
@@ -40,5 +40,16 @@ static constexpr std::array<swoc::TextView, 9> Severity_Names{
{"Diag", "Debug", "Status", "Note", "Warn", "Error", "Fatal", "Alert", "Emergency"}
};
+inline std::error_code
+ec_for()
+{
+ return {errno, std::system_category()};
+}
+inline std::error_code
+ec_for(int err)
+{
+ return {err, std::system_category()};
+}
+
Review Comment:
Nit: the name `ec_for` isn't very intuitive. IMHO naming this consistent with `make_error_code` would be more easily understandable.
--
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] SolidWallOfCode commented on pull request #9878: libswoc: Update Errata use in cache tool.
Posted by "SolidWallOfCode (via GitHub)" <gi...@apache.org>.
SolidWallOfCode commented on PR #9878:
URL: https://github.com/apache/trafficserver/pull/9878#issuecomment-1640975395
Had to update due to conflicts.
--
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] SolidWallOfCode commented on pull request #9878: libswoc: Update Errata use in cache tool.
Posted by "SolidWallOfCode (via GitHub)" <gi...@apache.org>.
SolidWallOfCode commented on PR #9878:
URL: https://github.com/apache/trafficserver/pull/9878#issuecomment-1596187444
```
/home/jenkins/workspace/Github_Builds/rocky/src@tmp/durable-351524f7/script.sh: line 4: /opt/rh/gcc-toolset-11/enable: No such file or directory
```
--
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] bneradt commented on pull request #9878: libswoc: Update Errata use in cache tool.
Posted by "bneradt (via GitHub)" <gi...@apache.org>.
bneradt commented on PR #9878:
URL: https://github.com/apache/trafficserver/pull/9878#issuecomment-1598025018
[approve ci rocky]
--
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] jpeach commented on a diff in pull request #9878: libswoc: Update Errata use in cache tool.
Posted by "jpeach (via GitHub)" <gi...@apache.org>.
jpeach commented on code in PR #9878:
URL: https://github.com/apache/trafficserver/pull/9878#discussion_r1266064448
##########
include/tscpp/util/ts_errata.h:
##########
@@ -40,5 +40,16 @@ static constexpr std::array<swoc::TextView, 9> Severity_Names{
{"Diag", "Debug", "Status", "Note", "Warn", "Error", "Fatal", "Alert", "Emergency"}
};
+inline std::error_code
+ec_for()
+{
+ return {errno, std::system_category()};
Review Comment:
And that that text continues:
```
It is also required to override the virtual function [std::error_category::default_error_condition()](https://en.cppreference.com/w/cpp/error/error_category/default_error_condition) to map the error codes that match POSIX [errno](https://en.cppreference.com/w/cpp/error/errno) values to [std::generic_category](https://en.cppreference.com/w/cpp/error/generic_category).
```
So, this means that system category is required to forward errno values to generic category? TBH this error abstraction seems to provide more confusion that value 🤷
##########
include/tscpp/util/ts_errata.h:
##########
@@ -40,5 +40,16 @@ static constexpr std::array<swoc::TextView, 9> Severity_Names{
{"Diag", "Debug", "Status", "Note", "Warn", "Error", "Fatal", "Alert", "Emergency"}
};
+inline std::error_code
+ec_for()
+{
+ return {errno, std::system_category()};
Review Comment:
And that that text continues:
```
It is also required to override the virtual function [std::error_category::default_error_condition()](https://en.cppreference.com/w/cpp/error/error_category/default_error_condition) to map the error codes that match POSIX [errno](https://en.cppreference.com/w/cpp/error/errno) values to [std::generic_category](https://en.cppreference.com/w/cpp/error/generic_category).
```
So, this means that system category is required to forward errno values to generic category? TBH this error abstraction seems to provide more confusion that value 🤷
--
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] bneradt commented on a diff in pull request #9878: libswoc: Update Errata use in cache tool.
Posted by "bneradt (via GitHub)" <gi...@apache.org>.
bneradt commented on code in PR #9878:
URL: https://github.com/apache/trafficserver/pull/9878#discussion_r1237426931
##########
src/traffic_cache_tool/CacheTool.cc:
##########
@@ -1349,7 +1349,7 @@ Scan_Cache(swoc::file::path const ®ex_path)
Cache cache;
std::vector<std::thread> threadPool;
if ((err = cache.loadSpan(SpanFile))) {
- if (err.size()) {
+ if (err.length()) {
Review Comment:
Why a length check rather than `if (!err.is_ok()) {`
##########
src/traffic_cache_tool/CacheTool.cc:
##########
@@ -1424,7 +1424,7 @@ main(int argc, const char *argv[])
arguments.invoke();
}
- if (err.size()) {
+ if (err.length()) {
Review Comment:
Why a length check rather than if (!err.is_ok()) {
--
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] jpeach commented on a diff in pull request #9878: libswoc: Update Errata use in cache tool.
Posted by "jpeach (via GitHub)" <gi...@apache.org>.
jpeach commented on code in PR #9878:
URL: https://github.com/apache/trafficserver/pull/9878#discussion_r1251476438
##########
include/tscpp/util/ts_errata.h:
##########
@@ -40,5 +40,16 @@ static constexpr std::array<swoc::TextView, 9> Severity_Names{
{"Diag", "Debug", "Status", "Note", "Warn", "Error", "Fatal", "Alert", "Emergency"}
};
+inline std::error_code
+ec_for()
+{
+ return {errno, std::system_category()};
Review Comment:
Is this right? The docs say that errno is in generic_category
https://en.cppreference.com/w/cpp/error/generic_category
--
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