You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by GitBox <gi...@apache.org> on 2021/02/05 02:23:50 UTC
[GitHub] [trafficserver] masaori335 opened a new pull request #7488: Avoid -Warray-bounds on PROXY Protocol Builder
masaori335 opened a new pull request #7488:
URL: https://github.com/apache/trafficserver/pull/7488
Address #7487. No logic change, add asserts to satisfy gcc (8.3.1 and 9.3.1).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] masaori335 commented on a change in pull request #7488: Avoid -Warray-bounds on PROXY Protocol Builder
Posted by GitBox <gi...@apache.org>.
masaori335 commented on a change in pull request #7488:
URL: https://github.com/apache/trafficserver/pull/7488#discussion_r571717876
##########
File path: iocore/net/ProxyProtocol.cc
##########
@@ -62,6 +62,8 @@ constexpr uint16_t PPv2_ADDR_LEN_INET = 4 + 4 + 2 + 2;
constexpr uint16_t PPv2_ADDR_LEN_INET6 = 16 + 16 + 2 + 2;
constexpr uint16_t PPv2_ADDR_LEN_UNIX = 108 + 108;
+const ts::BWFSpec ADDR_ONLY_FMT{"::a"};
Review comment:
It'd be great if we can make this `constexpr`, but it's another story.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] SolidWallOfCode commented on pull request #7488: Avoid -Warray-bounds on PROXY Protocol Builder
Posted by GitBox <gi...@apache.org>.
SolidWallOfCode commented on pull request #7488:
URL: https://github.com/apache/trafficserver/pull/7488#issuecomment-773760508
It would be interesting to try
```
ts::bwformat(bw, ts::BWFSpec::DEFAULT, pp_info.src_addr);
```
and see if that changes the warning. It would avoid declaring `src_ip_buf` at all.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] SolidWallOfCode commented on pull request #7488: Avoid -Warray-bounds on PROXY Protocol Builder
Posted by GitBox <gi...@apache.org>.
SolidWallOfCode commented on pull request #7488:
URL: https://github.com/apache/trafficserver/pull/7488#issuecomment-774134807
There's some overhead to parsing but it should be small. You can pre-parse the format if you want, there's a mechanism for that. It's a reason I suggested using `bwformat` directly - that skips the parsing. The `BWFSpec` class does the parsing of the specifier so you could be declare something like
```
static BWFSpec const addr_only{"::a"};
bwformat(w, addr_only, pp_info.src_addr);
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] masaori335 merged pull request #7488: Avoid -Warray-bounds on PROXY Protocol Builder
Posted by GitBox <gi...@apache.org>.
masaori335 merged pull request #7488:
URL: https://github.com/apache/trafficserver/pull/7488
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] masaori335 commented on pull request #7488: Avoid -Warray-bounds on PROXY Protocol Builder
Posted by GitBox <gi...@apache.org>.
masaori335 commented on pull request #7488:
URL: https://github.com/apache/trafficserver/pull/7488#issuecomment-774787764
The pre-parsed format sounds good. I updated this with it. Please take another look.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] masaori335 commented on pull request #7488: Avoid -Warray-bounds on PROXY Protocol Builder
Posted by GitBox <gi...@apache.org>.
masaori335 commented on pull request #7488:
URL: https://github.com/apache/trafficserver/pull/7488#issuecomment-773789886
```
ts::bwformat(bw, {"{::a}"}, pp_info.src_addr);
```
or
```
IpAddr src{pp_info.src_addr};
ts::bwformat(bw, ts::BWFSpec::DEFAULT, src);
```
is possible. But these "format" functions have some overheads of parsing format, right?
If we can use `ts::bwformat`, probably we can replace whole this function like
```
bw.print("{0} {1} {2::a} {3::a} {2::p} {3::p}\r\n", PPv1_CONNECTION_PREFACE, fam, pp_info.src_addr, pp_info.dst_addr);
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] masaori335 commented on pull request #7488: Avoid -Warray-bounds on PROXY Protocol Builder
Posted by GitBox <gi...@apache.org>.
masaori335 commented on pull request #7488:
URL: https://github.com/apache/trafficserver/pull/7488#issuecomment-773789886
```
ts::bwformat(bw, {"{::a}"}, pp_info.src_addr);
```
or
```
IpAddr src{pp_info.src_addr};
ts::bwformat(bw, ts::BWFSpec::DEFAULT, src);
```
is possible. But these "format" functions have some overheads of parsing format, right?
If we can use `ts::bwformat`, probably we can replace whole this function like
```
bw.print("{0} {1} {2::a} {3::a} {2::p} {3::p}\r\n", PPv1_CONNECTION_PREFACE, fam, pp_info.src_addr, pp_info.dst_addr);
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] SolidWallOfCode commented on pull request #7488: Avoid -Warray-bounds on PROXY Protocol Builder
Posted by GitBox <gi...@apache.org>.
SolidWallOfCode commented on pull request #7488:
URL: https://github.com/apache/trafficserver/pull/7488#issuecomment-773760508
It would be interesting to try
```
ts::bwformat(bw, ts::BWFSpec::DEFAULT, pp_info.src_addr);
```
and see if that changes the warning. It would avoid declaring `src_ip_buf` at all.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] zwoop commented on pull request #7488: Avoid -Warray-bounds on PROXY Protocol Builder
Posted by GitBox <gi...@apache.org>.
zwoop commented on pull request #7488:
URL: https://github.com/apache/trafficserver/pull/7488#issuecomment-784381493
Cherry-picked to v9.1.x branch.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org