352929 - Remove unused function DER_Decode

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P3)

Description

[Wan-Teh Chang]

The function DER_Decode is not exported and is not used
by NSS itself.  We should remove it.

Like SEC_ASN1DecodeItem, DER_Decode also allows extra input
after the ASN.1 object in the buffer.  This can be fixed by
testing buf_len == header_len + contents_len after the
der_capture call.  But it doesn't make sense to fix dead
code.

We can use the QuickDER decoder to decode DER.  It's not
necessary to keep DER_Decode.

Comment 1

[Biswatosh Chakraborty]

Attachment: Removes DER_Decode from nss/lib/util/derdec.c

Removes also der_decode() from derdec.c.
der_decode() is either called by itself only or by 
DER_Decode(). 

Other functions in the same file are called from other places
in nss.
SEC_QuickDERDecodeItem() is being used in der decoding in lot of
places in NSS. Plus, indeed, as Wan-Teh Chang noted, DER_Decode()
is also not exported outside. 

Hence, we may remove these two functions.

Comment 2

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 282393 [details] [diff] [review]
Removes DER_Decode from nss/lib/util/derdec.c 

r- This patch is incomplete.
The function declaration needs to be removed from header files, also.

Comment 3

[Biswatosh Chakraborty]

Attachment: Patch ver 2. Now removes function declaration from header file also.

Removes DER_Decode() declaration from nss/lib/util/secder.h
,in addition to what the previous patch (282393) was already doing.

Comment 4

[Wan-Teh Chang]

Comment on attachment 282497 [details] [diff] [review]
Patch ver 2.  Now removes function declaration from header file also.

r=wtc.  Please delete the blank lines before
the code you deleted so that blocks of code are
separated by one blank line.  Thanks!

It would be nice to see if DER_Length can be merged
with the QuickDER decoder.  That would be another bug.

Comment 5

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 282497 [details] [diff] [review]
Patch ver 2.  Now removes function declaration from header file also.

r=nelson

Comment 6

[Biswatosh Chakraborty]

Committing to nss/lib/util/derdec.c and secder.h to NSSBranch 

Checking in derdec.c;
/cvsroot/mozilla/security/nss/lib/util/derdec.c,v  <--  derdec.c
new revision: 1.5; previous revision: 1.4
done

Checking in secder.h;
/cvsroot/mozilla/security/nss/lib/util/secder.h,v  <--  secder.h
new revision: 1.10; previous revision: 1.9
done

Comment 7

[Biswatosh Chakraborty]

A mistake in the comment.
I have commited to NSSHead and not to Branch.
cvs commit comment also should have mentioned Head.
Sorry for that. :(

Comment 8

[Biswatosh Chakraborty]

This time checking to the Branch(NSS_3_11_BRANCH)...

Checking in derdec.c;
/cvsroot/mozilla/security/nss/lib/util/derdec.c,v  <--  derdec.c
new revision: 1.3.28.1; previous revision: 1.3
done

Checking in secder.h;
/cvsroot/mozilla/security/nss/lib/util/secder.h,v  <--  secder.h
new revision: 1.7.28.2; previous revision: 1.7.28.1
done

Comment 9

[Biswatosh Chakraborty]

Commited the changes both to the trunk and to the branch.
Marking this bug as resolved fixed.

Comment 10

[Wan-Teh Chang]

Biswatosh, thank you for fixing this bug.

It is not necessary to fix noncritical bugs on the NSS_3_11_BRANCH.
This bug's severity is only "trivial".

Since you checked in the fix on the NSS_3_11_BRANCH, please update
the target milestone to the next NSS 3.11.x release.

Comment 11

[Biswatosh Chakraborty]

Although cvs commit opened up the editor and I had written comments
in it but it did not succeed for some reason, when I saw it later. 
Hence, backed out the commits for derdec.c both in Branch and Trunk and again
recommited them back with appropriate comments.





The following details are just for reference log for backing out
and recommiting back the new version of derdec.c.

********************************************************************
For Branch.

Backing out...
cvs update -j1.3.28.1 -j1.3 mozilla/security/nss/lib/util/derdec.c
RCS file: /cvsroot/mozilla/security/nss/lib/util/derdec.c,v
retrieving revision 1.3.28.1
retrieving revision 1.3
Merging differences between 1.3.28.1 and 1.3 into derdec.c
[root@localhost util]# cvs commit derdec.c
Checking in derdec.c;
/cvsroot/mozilla/security/nss/lib/util/derdec.c,v  <--  derdec.c
new revision: 1.3.28.2; previous revision: 1.3.28.1
done
Backout done.

Recommiting ...
Checking in derdec.c;
/cvsroot/mozilla/security/nss/lib/util/derdec.c,v  <--  derdec.c
new revision: 1.3.28.3; previous revision: 1.3.28.2
done
Recommited back derdec.c to the Branch.
*****************************************************


For Head:

Backing out...
cvs update -j1.5 -j1.4 mozilla/security/nss/lib/util/derdec.c
RCS file: /cvsroot/mozilla/security/nss/lib/util/derdec.c,v
retrieving revision 1.5
retrieving revision 1.4
Merging differences between 1.5 and 1.4 into derdec.c
Checking in derdec.c;
/cvsroot/mozilla/security/nss/lib/util/derdec.c,v  <--  derdec.c
new revision: 1.6; previous revision: 1.5
done
Backout done.

Reecommiting derdec.c to the HEAD.

[root@localhost util]# cvs commit derdec.c
Checking in derdec.c;
/cvsroot/mozilla/security/nss/lib/util/derdec.c,v  <--  derdec.c
new revision: 1.7; previous revision: 1.6
done

Recommited back derdec.c to the Head.
***************************************

Comment 12

[Biswatosh Chakraborty]

Since now, it is commited to the branch, setting the target to 3.11.8