Patchwork scripts/test-installation.pl: Handle NSS crypto libraries [BZ #21940]

login
register
mail settings
Submitter Florian Weimer
Date Oct. 10, 2017, 9:26 a.m.
Message ID <81d2608e-6313-caeb-9667-28103387d852@redhat.com>
Download mbox | patch
Permalink /patch/23423/
State New
Headers show

Comments

Florian Weimer - Oct. 10, 2017, 9:26 a.m.
On 10/07/2017 12:50 PM, Rical Jasan wrote:

> I would use:
> 
> /^\s*lib(\w+)\.so(?:\.([0-9\.]+))?\s*=>.*\.so(?:\.([0-9\.]+))?/
>             ^                  ^                           ^
> to avoid matching "lib.so.".

I made the change in the attached patch.

>>       $found{$name} = 1;
>> -    if ($versions{$name} ne $version1 || $version1 ne $version2) {
>> +    if (defined($version1) != defined($version2)
>> +	|| defined($version1) != defined($versions{$name})
>> +	|| (defined($versions{$name})
>> +	    && ($versions{$name} ne $version1 || $version1 ne $version2))) {
>>         print "Library lib$name is not correctly installed.\n";
>>         print "Please check your installation!\n";
>>         print "Offending line of ldd output: $_\n";
> 
> It might help readability to follow up the match with something like:
> 
>   next if ! (defined($name) && defined($version1) && defined($version2));
>   next if ! (exists $versions{$name}) && defined($versions{$name}));

Is this really clearer?  What I wanted to express is “error if the 
defined-ness state is different across the three value, and if 
$versions{$name} is defined, it must much both versions”.  The original 
condition captures this fairly succinctly.

> (I seem to recall tests on hash entries creating them when they didn't
> previously exist, but that may not matter here.  Something other than
> `next' may also be desirable, and the opportunity for more fine-grained
> error messages is introduced, if useful.)

I suspect the original script was written for Perl 4, which did not have 
the exists operator.  I instinctively stuck to that baseline.

Anyway, I came up with something simpler, which sidesteps these issues.

> Then you could retain the simpler conditional:
> 
>   if ($versions{$name} ne $version1 || $version1 ne $version2) {
> 
> That also continues using string comparisons, which would be better in
> case there are multiple "."'s.

I only used != as the Boolean XOR operator on the numeric result from 
the defined operator.

Thanks,
Florian

Patch


The warning looked like this:

Use of uninitialized value in string ne at
  …/scripts/test-installation.pl line 184, <LDD> line 24.

It is triggered by this line of ldd output:

 libfreebl3.so => /lib64/libfreebl3.so (0x00007f055003c000)

The other lines have a version in the soname:

 libanl.so.1 => /lib64/libanl.so.1 (0x00007f055023f000)

2017-10-10  Florian Weimer  <fweimer@redhat.com>

	[BZ #21940]
	* scripts/test-installation.pl: Handle NSS crypto libaries in ldd
	output.

diff --git a/scripts/test-installation.pl b/scripts/test-installation.pl
index 74d25e1c8d..7eaeb1ae06 100755
--- a/scripts/test-installation.pl
+++ b/scripts/test-installation.pl
@@ -176,10 +176,17 @@  open LDD, "ldd /tmp/test-prg$$ |"
   or die ("Couldn't execute ldd");
 while (<LDD>) {
   if (/^\s*lib/) {
-    ($name, $version1, $version2) =
-      /^\s*lib(\w*)\.so\.([0-9\.]*)\s*=>.*\.so\.([0-9\.]*)/;
+    # When libcrypt is linked against NSS, some of the referenced
+    # libraries do not have a trailing version in their soname.
+    my ($name, $version1, $version2) =
+      /^\s*lib(\w+)\.so(?:\.([0-9\.]+))?\s*=>.*\.so(?:\.([0-9\.]+))?/;
     $found{$name} = 1;
-    if ($versions{$name} ne $version1 || $version1 ne $version2) {
+    # Version strings are either undefined or non-empty.
+    $version1 = '' unless defined $version1;
+    $version2 = '' unless defined $version2;
+    my $vername = $versions{$name};
+    $vername = '' unless defined $vername;
+    if ($version1 ne $version2 || $vername ne $version1) {
       print "Library lib$name is not correctly installed.\n";
       print "Please check your installation!\n";
       print "Offending line of ldd output: $_\n";