sahvx655-wq commented on PR #3591: URL: https://github.com/apache/brpc/pull/3591#issuecomment-6075437194
Pushed a comment-only follow-up for the two bot findings that were actually right. The constant's comment no longer quotes the wire format as requiring a *positive* integer, because the check deliberately lets 0 through, and the test comment now mentions the negative entry alongside the oversized ones. Nothing else moved: the diff is two comment blocks, so the bound and the six new table entries behave exactly as they did when CI went green. I did not take the other two. Rejecting 0 would be a behaviour change in the wrong direction - `0S` currently yields a deadline of `gettimeofday_us() + 0`, already expired, whereas -1 means no deadline at all at that call site, so a peer asking for an immediate deadline would end up unbounded. And `static const` at file scope is what the other ~20 constants under `src/brpc` use, with `static` already giving internal linkage, so there is no ODR hazard to fix by switching this one site to `constexpr`. Re-checked the patched conversion standalone under `-fsanitize=undefined -fno-sanitize-recover=all`: all five overflow inputs return -1, `99999999H` tops out at 3.6e17, and `1H`/`4m`/`5u`/`6n`/`0S` are unchanged. -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
