Avoid string copies by using string_view instead of string. [chromium/src : main]
0 views
Skip to first unread message
Lambros Lambrou (Gerrit)
Jan 15, 2026, 1:59:33 PM (5 days ago) Jan 15
to Jeffrey Yu, AyeAye, Chromium LUCI CQ, chromotin...@chromium.org
Attention needed from Jeffrey Yu
Lambros Lambrou voted and added 2 comments![]( "Open in Gerrit")
Votes added by Lambros Lambrou
| Code-Review | +1 |
2 comments
File remoting/protocol/sdp_message.cc
Line 130, Patchset 4 (Latest):
const std::string_view line = sdp_lines_[i];Lambros Lambrou . resolved
I think `sdp_lines_` should be a vector of `std::string_view`, since it originally comes from splitting a string (SDP message) into lines. But that can be another CL 😊
Line 140, Patchset 4 (Latest):
std::string payload_type = std::string(Lambros Lambrou . unresolved
Is the extra `std::string()` needed here? Why not use:
```
std::string payload_type(line.substr...);
```
or keep the original code?
Related details
Attention is currently required from:
- Jeffrey Yu
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Jeffrey Yu (Gerrit)
Jan 15, 2026, 2:23:12 PM (5 days ago) Jan 15
to Lambros Lambrou, AyeAye, Chromium LUCI CQ, chromotin...@chromium.org
Attention needed from Lambros Lambrou
Jeffrey Yu added 1 comment![]( "Open in Gerrit")
File remoting/protocol/sdp_message.cc
Is the extra `std::string()` needed here? Why not use:
```
std::string payload_type(line.substr...);
```
or keep the original code?
Attention is currently required from:
- Lambros Lambrou
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Lambros Lambrou (Gerrit)
Jan 15, 2026, 2:25:30 PM (5 days ago) Jan 15
to Jeffrey Yu, AyeAye, Chromium LUCI CQ, chromotin...@chromium.org
Attention needed from Jeffrey Yu
Lambros Lambrou voted Code-Review+1![]( "Open in Gerrit")
Attention is currently required from:
- Jeffrey Yu
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Jeffrey Yu (Gerrit)
Jan 15, 2026, 3:07:21 PM (5 days ago) Jan 15
to Lambros Lambrou, AyeAye, Chromium LUCI CQ, chromotin...@chromium.org
Attention needed from Lambros Lambrou
Jeffrey Yu added 1 comment![]( "Open in Gerrit")
File remoting/protocol/sdp_message.cc
Line 140, Patchset 4:
std::string payload_type = std::string(Lambros Lambrou . resolved
Jeffrey YuIs the extra `std::string()` needed here? Why not use:
```
std::string payload_type(line.substr...);
```
or keep the original code?
Done
Jeffrey Yu
Inlined the statement, but the extra `std::string` is needed to convert the type from string_view to string to save into the struct.
Related details
Attention is currently required from:
- Lambros Lambrou
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Lambros Lambrou (Gerrit)
Jan 15, 2026, 4:16:33 PM (5 days ago) Jan 15
to Jeffrey Yu, AyeAye, Chromium LUCI CQ, chromotin...@chromium.org
Attention needed from Jeffrey Yu
Lambros Lambrou voted Code-Review+1![]( "Open in Gerrit")
Attention is currently required from:
- Jeffrey Yu
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Jeffrey Yu (Gerrit)
Jan 15, 2026, 4:28:49 PM (5 days ago) Jan 15
to Joe Downing, Lambros Lambrou, AyeAye, Chromium LUCI CQ, chromotin...@chromium.org
Attention needed from Joe Downing
Jeffrey Yu voted Auto-Submit+1![]( "Open in Gerrit")
Attention is currently required from:
- Joe Downing
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Joe Downing (Gerrit)
Jan 15, 2026, 4:29:27 PM (5 days ago) Jan 15
to Jeffrey Yu, Lambros Lambrou, AyeAye, Chromium LUCI CQ, chromotin...@chromium.org
Attention needed from Jeffrey Yu
Joe Downing voted Code-Review+1![]( "Open in Gerrit")
Attention is currently required from:
- Jeffrey Yu
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Jeffrey Yu (Gerrit)
Jan 15, 2026, 4:35:16 PM (5 days ago) Jan 15
to Joe Downing, Lambros Lambrou, AyeAye, Chromium LUCI CQ, chromotin...@chromium.org
Jeffrey Yu voted Commit-Queue+2![]( "Open in Gerrit")
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
Jan 15, 2026, 4:38:11 PM (5 days ago) Jan 15
to Jeffrey Yu, Joe Downing, Lambros Lambrou, AyeAye, chromotin...@chromium.org
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
Commit message:
Avoid string copies by using string_view instead of string.
Change-Id: I7abb78f24a5bc06747979a5db0c89abe8a23d579
Reviewed-by: Lambros Lambrou <lambros...@chromium.org>
Auto-Submit: Jeffrey Yu <yu...@chromium.org>
Commit-Queue: Jeffrey Yu <yu...@chromium.org>
Reviewed-by: Joe Downing <joe...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1569986}
Files:
- M remoting/protocol/sdp_message.cc
Change size: S
Delta: 1 file changed, 7 insertions(+), 6 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Joe Downing, +1 by Lambros Lambrou
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages