Bug 8777 - Quiet errors in circulation.pl
Summary: Quiet errors in circulation.pl
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Bugs List
QA Contact:
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-09-17 15:28 UTC by Owen Leonard
Modified: 2014-12-07 20:02 UTC (History)
3 users (show)

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


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Owen Leonard 2012-09-17 15:28:40 UTC
Reference found where even-sized list expected at /circ/circulation.pl line 97.
 Use of uninitialized value $print in string eq at /circ/circulation.pl line 161.
 Use of uninitialized value in string eq at /circ/circulation.pl line 166.
 Use of uninitialized value $print in string eq at /circ/circulation.pl line 173.
 Use of uninitialized value in string eq at /circ/circulation.pl line 364.
 Use of uninitialized value in string ne at /circ/circulation.pl line 382.
 Use of uninitialized value in string eq at /circ/circulation.pl line 388.
 Use of uninitialized value in string gt at /circ/circulation.pl line 464.
 Use of uninitialized value in string eq at /circ/circulation.pl line 461.
 Use of uninitialized value in hash element at /circ/circulation.pl line 644.
 Use of uninitialized value in concatenation (.) or string at /circ/circulation.pl line 644.
Comment 1 Mark Tompsett 2012-09-27 10:20:28 UTC
Good thing I went looking for a duplicate.

borrowernumber is a bit more of a problem, as it is harder to trace the logic as to whether it should be q{} or 0. Though, I think 0 would result in different behaviour than undefined in some cases. Any thoughts on this?

Regarding line 97, see bug 8826. -- I had put that already up, because some of these patches don't require specialized testing for plakified situations.

findborrower, barcode, and print can all be fixed with a || q{}
This should deal with line 161, 173

if ( $barcode eq '' and defined($query->param('charges')) and $query->param('charges') eq 'yes' ) {
should fix 166

Haven't encountered 364, but I suppose:
if ( defined($num_res->{'found'}) and $num_res->{'found'} eq 'W' ) {

Haven't encountered 382, but that would be an uglier check.
Haven't encountered 388, but I think a ! =~ \S would be in there somehow.

(!defined($it->{'author'}) or $it->{'author'} eq '') and $it->{'author'} = ' ';
I believe solves 461.

if ( (defined ($it->{'issuedate'}) and $it->{'issuedate'} gt $todaysdate) or (defined ($it->{'lastreneweddate'}) and $it->{'lastreneweddate'} gt $todaysdate )) {
should fix 464

Regarding the $address, I was thinking:
my $address = q{};
$address .= $borrower->{'streetnumber'} if $borrower->{'streetnumber'};
$address .= ' ' if $address and $borrower->{'streettype'};
$address .= $roadttype_hashref->{$borrower->{'streettype'}} if $borrower->{'streettype'};
$address .= ' ' if $address and $borrower->{'address'};
$address .= $borrower->{'address'} if $borrower->{'address'};
But I'm sure there is an optimization somehow. This addresses 644.

I haven't generated a patch for any of this yet, since I wouldn't want to step on any toes. Though, your thoughts regarding borrowernumber would be helpful.

I did confirm that sth->execute(undef variable) generates 0 just like sth->execute(empty variable) for a "SELECT COUNT(*) ..." as a result of looking through these error log entries.

These errors overlap into 3.8.x as well.
Comment 2 Owen Leonard 2012-09-28 13:41:42 UTC
(In reply to comment #1)
> I haven't generated a patch for any of this yet, since I wouldn't want to
> step on any toes.

I started working on this but got nowhere. I'm unassigning myself, so if you have something to submit please do.
Comment 3 Mark Tompsett 2012-10-05 13:22:48 UTC
bug 8880 should address lines 97, 161, and 173 (which have shifted in the various versions). This overlapped with bug 8826 which only dealt with line 97. The problem is the initializations of $findborrower is less than 3 lines away from the change in bug 8826. So, all changes in bug 8826's patch are included in bug 8880's patch.
Comment 4 Mark Tompsett 2014-01-20 05:00:18 UTC
I believe bug 11182 closed the final ones remaining.