Bug 8895 - Warning in systempreferences.pl: Use of uninitialized value in length...
Summary: Warning in systempreferences.pl: Use of uninitialized value in length...
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Staff interface (show other bugs)
Version: 3.8
Hardware: All All
: P5 - low minor (vote)
Assignee: Marc Véron
QA Contact: Paul Poulain
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-10-10 05:44 UTC by Marc Véron
Modified: 2014-05-26 21:04 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 8895 - Warning in systempreferences.pl: Use of uninitialized value in length... (1.84 KB, patch)
2012-10-10 05:54 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 8895 - Warning in systempreferences.pl: Use of uninitialized value in length... (1.90 KB, patch)
2012-10-12 14:39 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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 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