Bug 21036: Fix warnings from C4/Biblio
authorMarcel de Rooy <m.de.rooy@rijksmuseum.nl>
Thu, 5 Jul 2018 07:53:57 +0000 (09:53 +0200)
committerLucas Gass <lucas@bywatersolutions.com>
Sun, 26 May 2019 12:58:20 +0000 (12:58 +0000)
Use of uninitialized value $isbn in string ne at /usr/share/koha/prodclone/C4/Biblio.pm line 1794. (16.11 line number)
Trivial edit.

And these warnings from TransformHtmlToXml (with 16.11 line numbers):
Use of uninitialized value in substr at /usr/share/koha/prodclone/C4/Biblio.pm line 2527.
Use of uninitialized value in substr at /usr/share/koha/prodclone/C4/Biblio.pm line 2528.
substr outside of string at /usr/share/koha/prodclone/C4/Biblio.pm line 2528.
Indicator in 952 is empty at /usr/share/koha/prodclone/C4/Biblio.pm line 2534.

The last warning is not needed and can be removed.
Note that the code used the construct @$indicator[$j] for $$indicator[$j].
The first is an array slice. This worked in list context. But apparently
the second was meant to be used. And can be rewritten as $indicator->[$j]
which generally is considered more readable.
The code around indicator1/2 and ind1/2 is simplified. This change is applied
twice in the same sub.

Test plan:
Read the changes.
Run t/Biblio/TransformHtmlToXml.t

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
(cherry picked from commit 295e9b1054386cf188c5fc1eb4fd590e5c451513)
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
(cherry picked from commit 6413f40738185f5319c66b763cb5bedc6fba8745)

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

C4/Biblio.pm

index 793299a..24aaf16 100644 (file)
@@ -1575,7 +1575,7 @@ sub GetMarcISBN {
     my @marcisbns;
     foreach my $field ( $record->field($scope) ) {
         my $isbn = $field->subfield( 'a' );
-        if ( $isbn ne "" ) {
+        if ( $isbn && $isbn ne "" ) {
             push @marcisbns, $isbn;
         }
     }
@@ -2272,16 +2272,9 @@ sub TransformHtmlToXml {
         if ( ( @$tags[$i] ne $prevtag ) ) {
             $close_last_tag = 0;
             $j++ unless ( @$tags[$i] eq "" );
-            my $indicator1 = eval { substr( @$indicator[$j], 0, 1 ) };
-            my $indicator2 = eval { substr( @$indicator[$j], 1, 1 ) };
-            my $ind1       = _default_ind_to_space($indicator1);
-            my $ind2;
-            if ( @$indicator[$j] ) {
-                $ind2 = _default_ind_to_space($indicator2);
-            } else {
-                warn "Indicator in @$tags[$i] is empty";
-                $ind2 = " ";
-            }
+            my $str = ( $indicator->[$j] // q{} ) . '  '; # extra space prevents substr outside of string warn
+            my $ind1 = _default_ind_to_space( substr( $str, 0, 1 ) );
+            my $ind2 = _default_ind_to_space( substr( $str, 1, 1 ) );
             if ( !$first ) {
                 $xml .= "</datafield>\n";
                 if (   ( @$tags[$i] && @$tags[$i] > 10 )
@@ -2314,19 +2307,12 @@ sub TransformHtmlToXml {
                 }
             }
         } else {    # @$tags[$i] eq $prevtag
-            my $indicator1 = eval { substr( @$indicator[$j], 0, 1 ) };
-            my $indicator2 = eval { substr( @$indicator[$j], 1, 1 ) };
-            my $ind1       = _default_ind_to_space($indicator1);
-            my $ind2;
-            if ( @$indicator[$j] ) {
-                $ind2 = _default_ind_to_space($indicator2);
-            } else {
-                warn "Indicator in @$tags[$i] is empty";
-                $ind2 = " ";
-            }
             if ( @$values[$i] eq "" ) {
             } else {
                 if ($first) {
+                    my $str = ( $indicator->[$j] // q{} ) . '  '; # extra space prevents substr outside of string warn
+                    my $ind1 = _default_ind_to_space( substr( $str, 0, 1 ) );
+                    my $ind2 = _default_ind_to_space( substr( $str, 1, 1 ) );
                     $xml .= "<datafield tag=\"@$tags[$i]\" ind1=\"$ind1\" ind2=\"$ind2\">\n";
                     $first = 0;
                     $close_last_tag = 1;