Copilot commented on code in PR #12900:
URL: https://github.com/apache/trafficserver/pull/12900#discussion_r2830570726
##########
src/proxy/logging/LogAccess.cc:
##########
@@ -626,6 +626,41 @@ LogAccess::unmarshal_int_to_str(char **buf, char *dest,
int len)
return -1;
}
+/*-------------------------------------------------------------------------
+ LogAccess::unmarshal_milestone_diff
+
+ Unmarshal a milestone difference value. Returns "-" when the
+ marshalled value is negative (milestone was unset), otherwise
+ the decimal string representation.
+ -------------------------------------------------------------------------*/
+
+int
+LogAccess::unmarshal_milestone_diff(char **buf, char *dest, int len)
+{
+ ink_assert(buf != nullptr);
+ ink_assert(*buf != nullptr);
+ ink_assert(dest != nullptr);
+
+ int64_t val = unmarshal_int(buf);
+ if (val < 0) {
+ if (len >= 1) {
Review Comment:
`unmarshal_milestone_diff()` treats any negative value as an unset milestone
and emits "-". However, `TransactionMilestones::difference_msec()` can also
return negative values when both milestones are set but the start/end order is
reversed (end-start < 0), so this would hide legitimate negative durations.
Consider mapping only the "missing" sentinel (currently -1 from
`difference_msec`'s default) to "-", and leaving other negative values as
numeric output so mis-ordered milestones remain debuggable.
--
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]
