295291 - certdata.perl generates a warning about a variable "used only once"

Status: RESOLVED FIXED

(NSS :: Build, defect, P2)

Description

[Wan-Teh Chang]

In mozilla/security/nss/lib/ckfw/builtins, if you
say
    gmake generate

you get the following warning message:

Name "main::a" used only once: possible typo at certdata.perl line 214.

    214 while(($a,$b) = each(%constants)) {
    215   print CFILE $b;
    216 }

This is because the variable 'a' is defined but not used.

Comment 1

[Wan-Teh Chang]

Attachment: A patch

This patch fixes the Perl interpreter warning.

I'd like to take the opportunity to fix another
problem with the script: the elements %constants
are stored in some random order, so each run of
this script may generate different output which
only differs in the ordering of the definition of
these constants.

Comment 2

[Wan-Teh Chang]

Attachment: Patch v1.1 (checked in)

One way to fix the certdata.c "stability" problem is
this patch.  We sort the constants.

Comment 3

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 189678 [details] [diff] [review]
Patch v1.1 (checked in)

While we're looking at this, we might as well fix ALL the issues with 
using this perl script to generate certdata.c from certdata.txt via 
   gmake generate

Another issue is that this perl script always produces output with Unix 
line endings, even when run on Windows.  The output (which is c source) 
must be converted to have Windows line endings before being compiled on 
Windows, and ESPECIALLY before being checked in from a windows system.  

Do we require users to use "nstools"?
Does nstools include the "flip" program?
If so, then we can simply add "flip -d certdata.c" to the makefile, 
after it runs perl.

Comment 4

[Wan-Teh Chang]

Attachment: Fix the problem with "chomp" in Cygwin's perl

I checked in "patch v1.1" on the NSS trunk (NSS 3.12).

/cvsroot/mozilla/security/nss/lib/ckfw/builtins/certdata.perl,v  <--  certdata.p
erl
new revision: 1.11; previous revision: 1.10
done

Nelson, the problem with line endings is not exactly
what you described.  Windows CVS and compilers can handle
Unix text files just fine.  The problem is the "chomp"
command of Cygwin's perl because it only removes Unix
text file line endings, leaving the CR ('\r') characters
behind.  So if you run "gmake generate" on Windows with
Cygwin's perl, the generated certdata.c file has some
extraneous CR characters (because the input file
certdata.txt is a Windows text file).  This patch
fixes the problem by not using "chomp".

Comment 5

[Nelson Bolyard (seldom reads bugmail)]

Wan-Teh, now that you've checked in the "patch v1.1", you should also rebuild
certdata.c with it and check that in, too, so that the comitted certdata.c 
source will be in sync with the committed certdata.perl script.  Otherwise,
people will wonder why the output they get from certdata.perl doesn't match 
what's checked in.  And IINM, that was one point of the patch, the make the 
output deterministicly reproducible.

Also, as you know, I'm deathly alergic to Cygwin, and don't use it.   :)
So, when I report that I get a certdata.c file with unix line endings, 
a patch for cygwin's perl may not help me.  But I will test that patch.

Also, I know that MSVC will happily compile source files with unix line 
endings but I'm less sure that cvs will happily check them in correctly.
I'll test that, too.  (Remember, no Cygwin involved here.)

Comment 6

[Wan-Teh Chang]

Nelson, I just regenerated certdata.c with the new certdata.perl
script and checked that in.

Checking in certdata.c;
/cvsroot/mozilla/security/nss/lib/ckfw/builtins/certdata.c,v  <--  certdata.c
new revision: 1.39; previous revision: 1.38
done

Comment 7

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 229486 [details] [diff] [review]
Fix the problem with "chomp" in Cygwin's perl

Sorry, I can't review this because I'm not a cygwin user.

Comment 8

[Wan-Teh Chang]

I need a volunteer to review attachment 229486 [details] [diff] [review], a patch for the
Perl script certdata.perl.

The issue is line endings.  The Perl command 'chomp' is supposed
to remove the line endings, but Cygwin's Perl does not handle a
Windows text file line ending, so this patch replaces 'chomp'
with a substitution command that removes both \r and \n.

Comment 9

[Nelson Bolyard (seldom reads bugmail)]

I propose that we resolve this bug as fixed in NSS 3.12.
Patch v1.1 was committed on the trunk for NSS 3.12, but 
was not carried back to the 3.11.x branch.
The workaround for the cygwin perl bug was never committed, but 
cygwin has fallen into disuse, I think.  
Mozilla users now use something else, such as MSYS.  
I don't know if this chomp problem is a problem with MSYS or not. 
It is not a problem with MKS.

Julien, since you use MSYS, can you check and see if the output of this 
perl script on MSYS has Windows line endings?  
If so, then I think we should declare victory and move on.

Comment 10

[Wan-Teh Chang]

Patch v1.1 (attachment 189678 [details] [diff] [review]) was carried back to the
NSS_3_11_BRANCH (NSS 3.11.9) along with the fix for
bug 402285.

Under MSYS, certdata.perl generates a certdata.c file
that has no extraneous CR characters, but this is probably
because the input certdata.txt file is a Unix text file under
MSYS in the first place.