ywkaras commented on code in PR #9620:
URL: https://github.com/apache/trafficserver/pull/9620#discussion_r1182059739
##########
proxy/http/HttpTransact.h:
##########
@@ -605,10 +605,9 @@ class HttpTransact
}
void
- increment(MgmtInt configured_connect_attempts_max_retries)
+ increment()
{
++_v;
- ink_assert(_v <= configured_connect_attempts_max_retries);
Review Comment:
The assert can be triggered if parent_connect_attempts >
(connect_attempts_max_retries + 1). If you can find checking logic in the
config code that enforces this, it would be safe to restore the assert.
This was the original version of increment():
```
void
increment()
{
++_v;
ink_assert(_v <= configured_connect_attempts_max_retries());
}
```
This was CurrentInfo::configured_connect_attempts_max_retries():
```
inline MgmtInt
HttpTransact::CurrentInfo::Attempts::configured_connect_attempts_max_retries()
const
{
return reinterpret_cast<const HttpTransact::State
*>(reinterpret_cast<const char *>(this) -
offsetof(HttpTransact::State, current.attempts))
->configured_connect_attempts_max_retries();
}
```
Even though this code was verified in Yahoo prod, and this sort of pointer
arithmetic is how inheritance is implemented in C++, this code was rejected in
review. I never would have put in the assert if I thought in would require
adding a parameter to increment().
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
