Open
Bug 400915
Opened 17 years ago
Updated 1 year ago
Remove all use of error strings from PKIX library code
Categories
(NSS :: Libraries, defect, P2)
NSS
Libraries
Tracking
(Not tracked)
NEW
People
(Reporter: nelson, Unassigned)
References
Details
(Whiteboard: PKIX)
Attachments
(2 files)
15.52 KB,
patch
|
alvolkov.bgs
:
review-
|
Details | Diff | Splinter Review |
6.59 KB,
patch
|
alvolkov.bgs
:
review+
|
Details | Diff | Splinter Review |
We've removed error strings from the PKIX_Error struct, having replaced them all with error numbers. Now it's time to remove all the code that carries those strings around inside of libPKIX library executable code. Then we'll remove the big table of strings itself from the shared library.
Reporter | ||
Comment 1•17 years ago
|
||
This patch removes all but two references to PKIX_ErrorText[] from the source code of the PKIX shared libs. It is not a complete removal of all PKIX error strings, but it is a major step. I'd like to commit this (or something like it) before going the next step. The reviewer will notice that there are numerous places where the old code detected an error in the fatal error class, and changed the error string, but did not change the error number. This patch removes the code that changed the error string, but does not insert new code to change the error number. Please advise if that is acceptable, or not.
Attachment #285973 -
Flags: review?(alexei.volkov.bugs)
Reporter | ||
Comment 2•17 years ago
|
||
In some sense, the fact that the unpatched code sets error strings, and does not also change the error number at the same time is a bug. Perhaps we should change the code to set an error code variable, but which one? Having multiple error code variables is quite confusing.
Priority: -- → P2
Reporter | ||
Comment 3•17 years ago
|
||
When I went to build the first patch, I was shocked at some of the warnings I was getting. Some test callback functions just didn't have the right sets of arguments. This patch fixes that, and also cleans up some other warnings. I could file a separate bug for this, but I thought I'd just tack it on to this bug. Hope you don't mind. :)
Attachment #285974 -
Flags: review?(alexei.volkov.bugs)
Reporter | ||
Comment 4•17 years ago
|
||
Comment on attachment 285974 [details] [diff] [review] cleanup function signature mismatches and other warnings in PKIX test commands The following new comment obviously needs to be fixed. > { >+ // needs work > return(NULL); > }
Comment 5•17 years ago
|
||
Comment on attachment 285973 [details] [diff] [review] remove error strings from pkix shared libs, part 1 r=alexei
Attachment #285973 -
Flags: review?(alexei.volkov.bugs) → review+
Comment 6•17 years ago
|
||
Comment on attachment 285974 [details] [diff] [review] cleanup function signature mismatches and other warnings in PKIX test commands r=alexei. Unfortunately we can not verify trough libpkix test run that changes are ok. These tests are still broken.
Attachment #285974 -
Flags: review?(alexei.volkov.bugs) → review+
Comment 7•17 years ago
|
||
Comment on attachment 285973 [details] [diff] [review] remove error strings from pkix shared libs, part 1 Changing from r+ to r- with the following reason: Some function that this patch touches(example pkix_Build_VerifyCertificate) customize the way of error handling. Example of the code, @@ -1143,23 +1141,18 @@ pkix_Build_VerifyCertificate( (pkixErrorResult, &pkixErrorClass, plContext); if (pkixTempResult) return pkixTempResult; if (pkixErrorClass == PKIX_FATAL_ERROR) { - pkixErrorMsg = PKIX_ErrorText[ - PKIX_USERCHECKERCHECKFAILED]; PKIX_RETURN(FATAL); if error is from fatal class, the function creates another error with new error string (PKIX_USERCHECKERCHECKFAILED) and makes error that was returned to be a cause of the new error. The with this patch modification new error will not get created, but old one will still be propagated to the upper level function. I don't particularly think that is it too harmful, but it is changing original intention of wrapping the error with a new error from cert chain building function. Also, we may change a class of the error at those level, and make it non-fatal. But this is should be done in the future. Thx to Nelson for noticing this bug.
Attachment #285973 -
Flags: review+ → review-
Reporter | ||
Updated•16 years ago
|
Reporter | ||
Comment 8•15 years ago
|
||
Unsetting target milestone in unresolved bugs whose targets have passed.
Target Milestone: 3.12 → ---
Reporter | ||
Updated•15 years ago
|
Whiteboard: PKIX
Comment 9•2 years ago
|
||
The bug assignee is inactive on Bugzilla, and this bug has priority 'P2'.
:beurdouche, could you have a look please?
For more information, please visit auto_nag documentation.
Assignee: nelson → nobody
Flags: needinfo?(bbeurdouche)
Updated•2 years ago
|
Severity: normal → S3
Comment 10•1 year ago
|
||
We have modified the bot to only consider P1 as high priority, so I'm cancelling the needinfo here.
Flags: needinfo?(bbeurdouche)
You need to log in
before you can comment on or make changes to this bug.
Description
•