Bug 8895

Summary: Warning in systempreferences.pl: Use of uninitialized value in length...
Product: Koha Reporter: Marc Véron <veron>
Component: Staff interfaceAssignee: Marc Véron <veron>
Status: CLOSED FIXED QA Contact: Paul Poulain <paul.poulain>
Severity: minor    
Priority: P5 - low CC: chris, gmcharlt, jonathan.druart, kyle, paul.poulain
Version: 3.8   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:
Attachments: Bug 8895 - Warning in systempreferences.pl: Use of uninitialized value in length...
Bug 8895 - Warning in systempreferences.pl: Use of uninitialized value in length...

Description Marc Véron 2012-10-10 05:44:38 UTC
Clicking on Local use tab in system preferences results in warning in log file:

systempreferences.pl: Use of uninitialized value in length at (...)/admin/systempreferences.pl line 456

Reason: content field "value" in table "systempreferences" can be NULL (and is tested for length).

Patch follows.
Comment 1 Marc Véron 2012-10-10 05:54:16 UTC Comment hidden (obsolete)
Comment 2 Kyle M Hall (khall) 2012-10-12 14:39:25 UTC
Created attachment 12789 [details] [review]
Bug 8895 - Warning in systempreferences.pl: Use of uninitialized value in length...

Clicking on Local use tab in system preferences results in warning in log file:

systempreferences.pl: Use of uninitialized value in length at (...)/admin/systempreferences.pl line 456

Reason: content field "value" in table "systempreferences" can be NULL (and is tested for length).

Added test for "defined" at to places.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 3 Paul Poulain 2012-10-25 13:09:04 UTC
QA comment: 2lines patch, add some variable check

passed QA
Comment 4 Paul Poulain 2012-10-25 13:11:45 UTC
Patch pushed because it's obviously correct, but on my test server, when I try to reach local use tab, I get a nasty:
Syck parser (line 53, column 0): syntax error at /usr/lib/perl5/YAML/Syck.pm line 126, <$fh> line 1.

(probably because i've no local use, but a nicer fallback would be welcomed !
Comment 5 Jonathan Druart 2012-10-25 13:43:08 UTC
QA comments:

1/ The line:
    unless (defined $data->{value}) { $data->{value} = "";}
could be wrote as:
    $data->{value} //= "";
(with Modern::Perl).

2/ The Bug 2505 collects this kind of patch :)

3/ The line 424 has a useless test if defined( $data->{value} )

Marked as Failed QA for 3
Comment 6 Jonathan Druart 2012-10-25 13:43:32 UTC
Collision... :)
Comment 7 Marc Véron 2012-10-25 14:06:21 UTC
Oh, 3) was already in the original code and not the target of my patch.
Comment 8 Paul Poulain 2012-10-25 14:11:15 UTC
(In reply to comment #5)
> 3/ The line 424 has a useless test if defined( $data->{value} )

I had some trouble understanding what you were meaning... and have got it now !

I've pushed the patch with a follow-up removing the useless code


(In reply to comment #7)
> Oh, 3) was already in the original code and not the target of my patch.

Marc, yes it was, but before your patch is was meaningfull. After your patch is was a dead code test, because the string has been defined 2 lines before.
Anyway, problem fixed !
Comment 9 Jonathan Druart 2012-10-25 14:12:57 UTC
(In reply to comment #7)
> Oh, 3) was already in the original code and not the target of my patch.

Yes but before the patch the test was not useless :) Now it is, the value will never undefined.

It's not a big deal! Anyway it is pushed :)
Comment 10 Jonathan Druart 2012-10-25 14:14:11 UTC
Definitely too slow today :)
Comment 11 Chris Cormack 2013-04-21 07:39:46 UTC
Pushed to 3.8.x will be in 3.8.12