SECU_PrintError should not print an empty error message when the error code is unknown.

RESOLVED FIXED in 3.4

Status

P2
normal
RESOLVED FIXED
17 years ago
17 years ago

People

(Reporter: wtc, Assigned: bishakhabanerjee)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

17 years ago
SECU_PrintError maintains a table error messages for the error
codes that it knows about.  If a new error code is added but
the error message table for SECU_PrintError is not updated,
SECU_PrintError will print nothing about the error.  This makes
debugging difficult.

I suggest that if SECU_PrintError can't find an error message
for the error code, it should print the error code in decimal.
For example, "error -5928".
(Reporter)

Updated

17 years ago
Priority: -- → P2
Target Milestone: --- → 3.4
(Reporter)

Comment 1

17 years ago
Assigned the bug to Bishakha.
Assignee: wtc → bishakhabanerjee
Target Milestone: 3.4 → 4.0
(Assignee)

Comment 2

17 years ago
Created attachment 68052 [details] [diff] [review]
patch

Created an attachment: patch
(Reporter)

Comment 3

17 years ago
Comment on attachment 68052 [details] [diff] [review]
patch


>-	fprintf(stderr, "\n");
>+	fprintf(stderr, ": %d\n", err);

It is better to day:
	fprintf(stderr, ": error %d\n", err);

Please attach an updated patch.  Thanks!
Attachment #68052 - Flags: needs-work+
(Assignee)

Comment 4

17 years ago
Created attachment 68161 [details] [diff] [review]
updated patch based on Wan-Teh's comments
(Reporter)

Comment 5

17 years ago
Comment on attachment 68161 [details] [diff] [review]
updated patch based on Wan-Teh's comments

r=wtc.

>-	fprintf(stderr, "\n");
>+	fprintf(stderr, ": error %d\n", err);

A nit: it is better to add a (int) cast in front of err:
	fprintf(stderr, ": error %d\n", (int)err);

This is because err is of the type PRErrorCode, which
is a PRInt32.  PRInt32 is not necessarily an int.
The %d format specifier requires an int, and compilers
don't do automatic type conversion for functions like
fprintf() and fscanf(), so it is safer to cast it to
an int to match the %d.

So, add the (int) cast and you can go ahead and check
it in.	Set the target milestone to 3.4, and mark the
bug RESOLVED.

Thanks.
Attachment #68161 - Flags: review+
(Assignee)

Comment 6

17 years ago
Created attachment 68449 [details] [diff] [review]
updated patch after furthur review by Wan-Teh


According to the checkin instructions on Mozilla, I waited till the tree was
green across all platforms, so I had to wait till this afternoon. When I went
to check in my change, I could not. I did not have permission to check into
mozilla/security. I have sent email against my CVS account creation bug
regarding that, but do not know when I will get a response. Meanwhile, to be
able to check this into target NSS 3.4:

Wan-Teh, Nelson,
could either of you please check in my change. I have built and verified
nothing breaks on Windows and Linux.

I will modify the bug appropriately after that and mark it RESOLVED
-Bishakha
(Reporter)

Comment 7

17 years ago
Comment on attachment 68449 [details] [diff] [review]
updated patch after furthur review by Wan-Teh

r=wtc.	Bishakha, please go ahead and check it in.
Remember to set target milestone to 3.4 and mark
the bug fixed.

To answer your questions:
1. Checkins to NSS are not subject to the Mozilla
client tree closure.  We are an independent group.
We only share the cvs repository and bug database
with them, but not much else.

Mozilla client pulls a stable snapshot of NSS (the
cvs tag is NSS_CLIENT_TAG), so the tip of NSS is
owned by our team.

2. There is cvs commit access control on the
mozilla/security directory.  I forgot to add you
to the access control list.  Sorry.  I just fixed
that.  You should be able to check in this patch
now.

Thanks.
Attachment #68449 - Flags: review+
(Assignee)

Comment 8

17 years ago
Checked in mozilla/security/nss/cmd/lib/secutil.c;
new revision: 1.32; previous revision: 1.31

Marking the bug "FIXED" and setting target to NSS 3.4
Status: NEW → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED
Target Milestone: 4.0 → 3.4
(Assignee)

Updated

17 years ago
Attachment #68052 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.