The default bug view has changed. See this FAQ.

Add a VERSIONINFO resource to nssckbi.dll.

RESOLVED FIXED in 3.9.3

Status

NSS
Libraries
RESOLVED FIXED
13 years ago
13 years ago

People

(Reporter: Wan-Teh Chang, Assigned: Bishakha Banerjee)

Tracking

3.9.3
x86
Windows XP

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

8.09 KB, patch
Robert Relyea
: review+
Nelson Bolyard (seldom reads bugmail)
: superreview+
Details | Diff | Splinter Review
(Reporter)

Description

13 years ago
nssckbi.dll doesn't have a VERSIONINFO resource
(defined in a .rc file).  We need to add it.
(Reporter)

Comment 1

13 years ago
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.
(Reporter)

Comment 2

13 years ago
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?
Attachment #153855 - Flags: review?(rrelyea0264)
(Reporter)

Updated

13 years ago
Target Milestone: --- → 3.9.3

Updated

13 years ago
Attachment #153855 - Flags: review?(rrelyea0264) → review+
(Reporter)

Comment 3

13 years ago
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").
Assignee: wchang0222 → bishakhabanerjee
(Reporter)

Comment 4

13 years ago
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
Attachment #153855 - Flags: superreview?(nelson)
(Reporter)

Comment 5

13 years ago
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 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.
Attachment #153855 - Flags: superreview?(nelson) → superreview+
(Reporter)

Comment 7

13 years ago
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".
(Reporter)

Comment 8

13 years ago
This has been fixed.
Status: NEW → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.