380334 - CERT_HTMLCertInfo is dead code

Status: RESOLVED FIXED

(NSS :: Libraries, defect)

Description

[Julien Pierre]

This function, implemented in lib/certhigh/certhtml.c, is not exported from nss.def, and not used in any cmd programs. So it is dead code. I propose deleting it.

Comment 1

[Julien Pierre]

Attachment: remove dead code

Comment 2

[Robert Relyea]

Comment on attachment 264417 [details] [diff] [review]
remove dead code

r+= relyea...

wow a refugee from the old sec/lib/ui or whatever we called the old html UI.

are we still using gatherStrings() as well and where is it?

bob

Comment 3

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 264417 [details] [diff] [review]
remove dead code

We cannot remove the function from the nss.def file.  Doing so would change the 
ordinals of the function addresses in the DLL's table of function pointers.
So, for backwards binary compatibility we need to retain the function in the 
nss.def file.  

Having the function name in the nss.def file will create an unsatisfied external
at link time when linking the DLL, unless there is an external function by that
name.  

So, the conclusion is: we can't remove the function completely.  The best we
can do is change the implementation to set an appropriate error code and then 
return NULL.

Comment 4

[Julien Pierre]

Comment on attachment 264417 [details] [diff] [review]
remove dead code

Nelson, this function was never in nss.def . My patch does not remove it from nss.def, only from certhtml.c .

Comment 5

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 264417 [details] [diff] [review]
remove dead code

Sorry, I see you even mentioned that it's not in nss.def in comment 0.  It's been a very long day.  :(

Comment 6

[Julien Pierre]

Attachment: Also remove the function from header file

Thanks, Nelson. Actually I also forgot to remove it from the header. Please review.

Comment 7

[Julien Pierre]

Bob,

Looks like we aren't using gatherStrings either. So that should be deleted as well. I will submit a consolidated patch.

I found this because I'm going through Slavo's coverage data and looking at areas with low test coverage, and dead code that's compiled in is showing up.

Comment 8

[Julien Pierre]

Attachment: also remove gatherStrings

3rd time should be the charm ;)

Comment 9

[Robert Relyea]

Comment on attachment 264437 [details] [diff] [review]
also remove gatherStrings

r+ = rrelyea

Comment 10

[Julien Pierre]

Checked in to the trunk :

Checking in cert.h;
/cvsroot/mozilla/security/nss/lib/certdb/cert.h,v  <--  cert.h
new revision: 1.56; previous revision: 1.55
done

Checking in certhtml.c;
/cvsroot/mozilla/security/nss/lib/certhigh/certhtml.c,v  <--  certhtml.c
new revision: 1.8; previous revision: 1.7
done

And to the 3.11 branch :
Checking in certdb/cert.h;
/cvsroot/mozilla/security/nss/lib/certdb/cert.h,v  <--  cert.h
new revision: 1.53.28.2; previous revision: 1.53.28.1
done
Checking in certhigh/certhtml.c;
/cvsroot/mozilla/security/nss/lib/certhigh/certhtml.c,v  <--  certhtml.c
new revision: 1.6.2.1; previous revision: 1.6
done