398019 - correct confusing and erroneous comments in DER_AsciiToTime

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P2)

Description

[Nelson Bolyard (seldom reads bugmail)]

Attachment: patch v1

While investigating bug 397968, I found that the source code for 
DER_AsciiToTime contains numerous comments that are confusing at best
or downright incorrect.  I also found uses of time_t that are 
unnecessary and harmless, but create confusion and suspicion.

The code appears to be entirely correct, but with the present comments
and odd type casts, it is difficult for a reviewer to SEE that it is 
correct without doing a lot of work.

So, I wrote a patch to correct the comments, and to eliminate the use
of time_t.  This patch does not change the actual computations or 
behavior any.  Its only purpose is to make the code more obviously 
correct to a reviewer.

Comment 1

[Nelson Bolyard (seldom reads bugmail)]

I need to correct one new comment line as follows:
- /* compute seconds in elapsed leap days since 1970 */
+ /* compute elapsed leap days since 1970 */

I need to think some more about whether that computation is correct for
years between 1950 and 1969.  I'm not sure that it is.  

Comment 2

[Robert Relyea]

Comment on attachment 282848 [details] [diff] [review]
patch v1

r+ with your comment update.

Comment 3

[Nelson Bolyard (seldom reads bugmail)]

Checking in lib/util/dertime.c; new revision: 1.9; previous revision: 1.8

Comment 4

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 282848 [details] [diff] [review]
patch v1

requesting second review for branch.  This patch is a precursor to the patch for bug 398693.

Comment 5

[Julien Pierre]

Comment on attachment 282848 [details] [diff] [review]
patch v1

This patch is correct, but for the new code, I would rather replace the old LL_I2L macro with something more readable, since it isn't needed anymore.

Comment 6

[Nelson Bolyard (seldom reads bugmail)]

I'm marking this bug fixed.  The patch ws committed on the trunk, but
not on the branch.  Then I discovered bug 398693 which completely 
rewrites these functions.  So, there remains no need for this bug's patch 
to be committed on the branch.