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

Issue 71009: Report what went wrong when initializing NSS_NoDB_Init. (Closed)

Created:
11 years, 8 months ago by Elliot Glaysher
Modified:
9 years, 7 months ago
Reviewers:
Dean McNamee
CC:
chromium-reviews_googlegroups.com
Visibility:
Public.

Description

Report what went wrong when initializing NSS_NoDB_Init. BUG=9857 Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=13602

Patch Set 1 #

Total comments: 4

Patch Set 2 : Fixes for deanm #

Unified diffs Side-by-side diffs Delta from patch set Stats (+10 lines, -2 lines) Patch
M base/nss_init.cc View 1 2 chunks +10 lines, -2 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
Elliot Glaysher
11 years, 8 months ago (2009-04-12 05:08:00 UTC) #1
Dean McNamee
LG http://codereview.chromium.org/71009/diff/1/2 File base/nss_init.cc (right): http://codereview.chromium.org/71009/diff/1/2#newcode48 Line 48: PRInt32 err_length = PR_GetErrorTextLength(); The length includes ...
11 years, 8 months ago (2009-04-12 09:27:21 UTC) #2
Elliot Glaysher
11 years, 8 months ago (2009-04-13 17:34:42 UTC) #3
http://codereview.chromium.org/71009/diff/1/2
File base/nss_init.cc (right):

http://codereview.chromium.org/71009/diff/1/2#newcode48
Line 48: PRInt32 err_length = PR_GetErrorTextLength();
On 2009/04/12 09:27:21, Dean McNamee wrote:
> The length includes space for the NULL byte?

Yes. The docs say "the value returned is sufficient to contain the error text
currently available," and all instances of the function on the first page of a
code search for the function don't add 1 for the NULL. (Most instances are in
Mozilla code, so I assume they know what they're doing.)

http://google.com/codesearch?q=PR_GetErrorTextLength&hl=en&btnG=Search+Code

http://codereview.chromium.org/71009/diff/1/2#newcode51
Line 51: }
On 2009/04/12 09:27:21, Dean McNamee wrote:
> don't need { }, but it's ok.

Done.

Powered by Google App Engine
This is Rietveld 408576698