Last Comment Bug 252375 - Add a VERSIONINFO resource to nssckbi.dll.
: Add a VERSIONINFO resource to nssckbi.dll.
Status: RESOLVED FIXED
:
Product: NSS
Classification: Components
Component: Libraries (show other bugs)
: 3.1
: x86 Windows XP
: -- normal (vote)
: 3.9.3
Assigned To: Bishakha Banerjee
: Bishakha Banerjee
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2004-07-20 18:34 PDT by Wan-Teh Chang
Modified: 2004-11-01 11:10 PST (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Proposed patch (8.09 KB, patch)
2004-07-20 19:46 PDT, Wan-Teh Chang
rrelyea: review+
nelson: superreview+
Details | Diff | Splinter Review

Description Wan-Teh Chang 2004-07-20 18:34:48 PDT
nssckbi.dll doesn't have a VERSIONINFO resource
(defined in a .rc file).  We need to add it.
Comment 1 Wan-Teh Chang 2004-07-20 19:46:13 PDT
Created attachment 153855 [details] [diff] [review]
Proposed patch

I added both a Windows VERSIONINFO resource (nssckbi.rc) and
SCCS and RCS version strings (ckbiver.c) that can be extracted
with the 'what' and 'ident' commands.

I am using nssckbi's own version (defined in nssckbi.h) rather
than the NSS version.

I describe the nssckbi library as "NSS Builtin Trusted Root CAs".
I am open to suggestions for a better or shorter description.

I added a new macro NSS_BUILTINS_LIBRARY_VERSION to nssckbi.h.

Note that in lib/ckfw/builtins/config.mk, the "OS2" in that
ifeq test is not necessary, so I deleted it.
Comment 2 Wan-Teh Chang 2004-07-20 19:52:47 PDT
Comment on attachment 153855 [details] [diff] [review]
Proposed patch

Bob, please review.

Also looking for suggestions on the name of the
new .c file containing SCCS and RCS version strings.
ckbiver.c, nssbiver.c, or nssckbiv.c?
Comment 3 Wan-Teh Chang 2004-07-26 21:55:26 PDT
Bishakha, please check in my patch on the trunk and the
3.9 branch.  On the 3.9 branch, the change for nssckbi.h
needs to made manually so that the NSS_BUILTINS_LIBRARY_VERSION
macro has the right value ("1.40").
Comment 4 Wan-Teh Chang 2004-08-31 10:31:00 PDT
Comment on attachment 153855 [details] [diff] [review]
Proposed patch

I checked in this patch on the trunk.

RCS file: /cvsroot/mozilla/security/nss/lib/ckfw/builtins/ckbiver.c,v
done
Checking in ckbiver.c;
/cvsroot/mozilla/security/nss/lib/ckfw/builtins/ckbiver.c,v  <--  ckbiver.c
initial revision: 1.1
done
Checking in config.mk;
/cvsroot/mozilla/security/nss/lib/ckfw/builtins/config.mk,v  <--  config.mk
new revision: 1.10; previous revision: 1.9
done
Checking in manifest.mn;
/cvsroot/mozilla/security/nss/lib/ckfw/builtins/manifest.mn,v  <--  manifest.mn
new revision: 1.9; previous revision: 1.8
done
Checking in nssckbi.h;
/cvsroot/mozilla/security/nss/lib/ckfw/builtins/nssckbi.h,v  <--  nssckbi.h
new revision: 1.9; previous revision: 1.8
done
RCS file: /cvsroot/mozilla/security/nss/lib/ckfw/builtins/nssckbi.rc,v
done
Checking in nssckbi.rc;
/cvsroot/mozilla/security/nss/lib/ckfw/builtins/nssckbi.rc,v  <--  nssckbi.rc
initial revision: 1.1
done
Comment 5 Wan-Teh Chang 2004-08-31 15:08:55 PDT
Comment on attachment 153855 [details] [diff] [review]
Proposed patch

I adapted this patch for the NSS_3_9_BRANCH and
checked it in.

Checking in ckbiver.c;
/cvsroot/mozilla/security/nss/lib/ckfw/builtins/ckbiver.c,v  <--  ckbiver.c
new revision: 1.1.2.1; previous revision: 1.1
done
Checking in config.mk;
/cvsroot/mozilla/security/nss/lib/ckfw/builtins/config.mk,v  <--  config.mk
new revision: 1.7.16.2; previous revision: 1.7.16.1
done
Checking in manifest.mn;
/cvsroot/mozilla/security/nss/lib/ckfw/builtins/manifest.mn,v  <--  manifest.mn
new revision: 1.7.16.1; previous revision: 1.7
done
Checking in nssckbi.h;
/cvsroot/mozilla/security/nss/lib/ckfw/builtins/nssckbi.h,v  <--  nssckbi.h
new revision: 1.6.16.1; previous revision: 1.6
done
Checking in nssckbi.rc;
/cvsroot/mozilla/security/nss/lib/ckfw/builtins/nssckbi.rc,v  <--  nssckbi.rc
new revision: 1.1.2.1; previous revision: 1.1
done
Comment 6 Nelson Bolyard (seldom reads bugmail) 2004-09-03 17:22:54 PDT
Comment on attachment 153855 [details] [diff] [review]
Proposed patch

sr += nelson

There is one aspect of this patch that I wish could be done slightly 
differently, but I'm not sure that it can.

>+#define NSS_BUILTINS_LIBRARY_VERSION "1.50"
> #define NSS_BUILTINS_LIBRARY_VERSION_MAJOR 1
> #define NSS_BUILTINS_LIBRARY_VERSION_MINOR 50

I wish that NSS_BUILTINS_LIBRARY_VERSION could be #defined as a 
pre-processor macro that depends on NSS_BUILTINS_LIBRARY_VERSION and
NSS_BUILTINS_LIBRARY_VERSION_MINOR, and expands to either "1.50" or 
to "1" "." "50".  I've tried numerous macros that use the # and ## 
operators, with no joy yet.
Comment 7 Wan-Teh Chang 2004-09-04 09:26:21 PDT
Nelson, thanks for the code review and comment.
Bob made the same comment about the version
string macro.  The reason I define a separate
macro for the version string is that it may need
to be free formed, for example, "1.41 Beta 2"
and "1.51 RC1".

It is possible to do what you suggested.  There
is a trick -- define two levels of macros. The
trick is used in nss.rc; look for the STRINGIZE
and STRINGIZE2 macros.

What I need you to review is the name of the
nssckbi module.  We have two names for it.
My patch calls it "NSS Builtin Trusted Root CAs".
We also call it "NSS Builtin Object Cryptoki
Module" in nss/lib/ckfw/builtins/constants.c.

FYI: Mozilla/PSM calls it "Builtin Roots Module".
AOL Communicator calls it "Built in Roots".
Comment 8 Wan-Teh Chang 2004-11-01 11:10:05 PST
This has been fixed.

Note You need to log in before you can comment on or make changes to this bug.