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

Issue 464064: Add StringPrintV, fix libxml_utils.cc to use it (Closed)

Created:
11 years ago by piman
Modified:
9 years, 7 months ago
Reviewers:
Nico
CC:
chromium-reviews_googlegroups.com
Visibility:
Public.

Description

Add StringPrintV, fix libxml_utils.cc to use it Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=34029

Patch Set 1 #

Total comments: 2

Patch Set 2 : adding va_end #

Total comments: 1

Patch Set 3 : remove wstring version of StringPrintV #

Unified diffs Side-by-side diffs Delta from patch set Stats (+11 lines, -1 line) Patch
M base/string_util.h View 1 2 1 chunk +3 lines, -0 lines 0 comments Download
M base/string_util.cc View 1 2 1 chunk +6 lines, -0 lines 0 comments Download
M chrome/common/libxml_utils.cc View 1 1 chunk +2 lines, -1 line 0 comments Download

Messages

Total messages: 4 (0 generated)
piman
11 years ago (2009-12-07 23:45:33 UTC) #1
Nico
http://codereview.chromium.org/464064/diff/1/4 File chrome/common/libxml_utils.cc (right): http://codereview.chromium.org/464064/diff/1/4#newcode38 chrome/common/libxml_utils.cc:38: reader->errors_.append(StringPrintV(msg, args)); This function is missing the corresponding va_end().
11 years ago (2009-12-07 23:50:36 UTC) #2
piman
http://codereview.chromium.org/464064/diff/1/4 File chrome/common/libxml_utils.cc (right): http://codereview.chromium.org/464064/diff/1/4#newcode38 chrome/common/libxml_utils.cc:38: reader->errors_.append(StringPrintV(msg, args)); On 2009/12/07 23:50:36, Nico wrote: > This ...
11 years ago (2009-12-08 00:04:02 UTC) #3
Nico
11 years ago (2009-12-08 00:13:43 UTC) #4
LG

http://codereview.chromium.org/464064/diff/4001/4002
File base/string_util.cc (right):

http://codereview.chromium.org/464064/diff/4001/4002#newcode1254
base/string_util.cc:1254: std::wstring StringPrintV(const wchar_t* format,
va_list ap) {
is the wchar_t version needed? If not, maybe leave it out. We're not supposed to
add new code that uses wchar_t I believe.

Powered by Google App Engine
This is Rietveld 408576698