Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(326)

Issue 3030035: Use built-in size function from std::string instead of strlen function. (Closed)

Created:
10 years, 4 months ago by tfarina
Modified:
9 years, 7 months ago
Reviewers:
Lei Zhang, viettrungluu
CC:
chromium-reviews, John Grabowski, jam, pam+watch_chromium.org, darin-cc_chromium.org, stuartmorgan+watch_chromium.org
Base URL:
git://git.chromium.org/chromium.git
Visibility:
Public.

Description

Use built-in size function from std::string instead of strlen function. This was missed in another review at http://codereview.chromium.org/3043018/diff/22001/23009#newcode28, noticed by viettrungluu. BUG=None TEST=trybots Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=55181

Patch Set 1 #

Total comments: 5

Patch Set 2 : trungl's review #

Total comments: 2

Patch Set 3 : rebased + review #

Unified diffs Side-by-side diffs Delta from patch set Stats (+6 lines, -7 lines) Patch
M chrome/plugin/plugin_main_mac.mm View 1 2 1 chunk +6 lines, -7 lines 0 comments Download

Messages

Total messages: 6 (0 generated)
tfarina
http://codereview.chromium.org/3030035/diff/1/2 File chrome/plugin/plugin_main_mac.mm (right): http://codereview.chromium.org/3030035/diff/1/2#newcode36 chrome/plugin/plugin_main_mac.mm:36: strcmp(interpose_list.c_str() + suffix_offset, how can I fix this trungl?
10 years, 4 months ago (2010-08-01 19:36:13 UTC) #1
tfarina
friendly ping.
10 years, 4 months ago (2010-08-03 03:16:16 UTC) #2
viettrungluu
http://codereview.chromium.org/3030035/diff/1/2 File chrome/plugin/plugin_main_mac.mm (right): http://codereview.chromium.org/3030035/diff/1/2#newcode28 chrome/plugin/plugin_main_mac.mm:28: int suffix_offset = static_cast<int>(interpose_list.size() - The suffix_offset better not ...
10 years, 4 months ago (2010-08-03 05:56:46 UTC) #3
tfarina
Please, take another look.
10 years, 4 months ago (2010-08-03 14:06:05 UTC) #4
viettrungluu
LGTM. http://codereview.chromium.org/3030035/diff/6001/7001 File chrome/plugin/plugin_main_mac.mm (right): http://codereview.chromium.org/3030035/diff/6001/7001#newcode30 chrome/plugin/plugin_main_mac.mm:30: size_t suffix_offset = interpose_list.size() - interpose_library_path.size(); Please DCHECK_GE(interpose_list.size(), ...
10 years, 4 months ago (2010-08-05 04:17:39 UTC) #5
tfarina
10 years, 4 months ago (2010-08-05 04:25:18 UTC) #6
http://codereview.chromium.org/3030035/diff/6001/7001
File chrome/plugin/plugin_main_mac.mm (right):

http://codereview.chromium.org/3030035/diff/6001/7001#newcode30
chrome/plugin/plugin_main_mac.mm:30: size_t suffix_offset =
interpose_list.size() - interpose_library_path.size();
On 2010/08/05 04:17:39, viettrungluu wrote:
> Please DCHECK_GE(interpose_list.size(), interpose_library_path.size());

Done.

Powered by Google App Engine
This is Rietveld 408576698