[logging] Remove redundant DCHECK in MessageBuilder [v8/v8 : main]
0 views
Skip to first unread message
Camillo Bruni (Gerrit)
Feb 23, 2026, 1:18:16 PM (20 hours ago) Feb 23
to Victor Gomes, v8-re...@googlegroups.com
Attention needed from Victor Gomes
Camillo Bruni added 1 comment![]( "Open in Gerrit")
Patchset-level comments
Related details
Attention is currently required from:
- Victor Gomes
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Victor Gomes (Gerrit)
2:42 AM (6 hours ago) 2:42 AM
to Camillo Bruni, V8 LUCI CQ, v8-re...@googlegroups.com
Attention needed from Camillo Bruni
Victor Gomes voted and added 2 comments![]( "Open in Gerrit")
Votes added by Victor Gomes
| Code-Review | +1 |
2 comments
Patchset-level comments
Victor Gomes . resolved
LGTM % comment
File src/logging/log-file.cc
Line 122, Patchset 2 (Latest):
AppendCharacter(static_cast<char>(c));Victor Gomes . unresolved
What about unicode characters? Why did you remove that?
We seem to escape non-printable ascii (latin1?) characters in LogFile::MessageBuilder::AppendCharacter, but not unicode.
Related details
Attention is currently required from:
- Camillo Bruni
Submit Requirements:
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Camillo Bruni (Gerrit)
8:00 AM (1 hour ago) 8:00 AM
to Victor Gomes, V8 LUCI CQ, v8-re...@googlegroups.com
Camillo Bruni voted and added 1 comment![]( "Open in Gerrit")
Votes added by Camillo Bruni
| Commit-Queue | +2 |
1 comment
File src/logging/log-file.cc
What about unicode characters? Why did you remove that?
We seem to escape non-printable ascii (latin1?) characters in LogFile::MessageBuilder::AppendCharacter, but not unicode.
Camillo Bruni
ah true, good call.
Related details
Attention set is empty
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
V8 LUCI CQ (Gerrit)
8:39 AM (26 minutes ago) 8:39 AM
to Camillo Bruni, Victor Gomes, v8-re...@googlegroups.com
V8 LUCI CQ submitted the change with unreviewed changes![]( "Open in Gerrit")
Unreviewed changes
2 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: src/logging/log-file.cc
Insertions: 6, Deletions: 1.
@@ -119,7 +119,12 @@
if (length_limit) length = std::min(length, *length_limit);
for (int i = 0; i < length; i++) {
uint16_t c = str->Get(i, access_guard);
- AppendCharacter(static_cast<char>(c));
+ if (c <= 0xFF) {
+ AppendCharacter(static_cast<char>(c));
+ } else {
+ // Escape non-ascii characters.
+ AppendRawFormatString("\\u%04x", c & 0xFFFF);
+ }
}
}
```
Change information
Commit message:
[logging] Remove redundant DCHECK in MessageBuilder
No need to check for the null-terminator since we already properly
escape it in LogFile::MessageBuilder::AppendCharacter.
Bug: 483599048
Change-Id: If14a21da1401791c9c42990cf251e384c8389643
Commit-Queue: Camillo Bruni <cbr...@chromium.org>
Reviewed-by: Victor Gomes <victo...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#105415}
Files:
- M src/logging/log-file.cc
Change size: XS
Delta: 1 file changed, 0 insertions(+), 2 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Victor Gomes
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages