Bug 4064 - Uninitialized variable errors in opac-search.pl
Summary: Uninitialized variable errors in opac-search.pl
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low normal (vote)
Assignee: Mark Tompsett
QA Contact: Jonathan Druart
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-01-26 05:19 UTC by Owen Leonard
Modified: 2013-12-05 19:59 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
Optimized two if statements, but otherwise fixed already in master. (1.48 KB, patch)
2012-06-30 05:52 UTC, Mark Tompsett
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 4064 - Uninitialized variable errors in opac-search.pl Merely optimized two if statements, already fixed in master. (1.49 KB, patch)
2012-08-06 13:37 UTC, Julian Maurice
Details | Diff | Splinter Review
Optimized two if statements (3.8.x) (1.65 KB, patch)
2012-09-04 00:37 UTC, Mark Tompsett
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Cormack 2010-05-21 01:22:44 UTC


---- Reported by oleonard@myacpl.org 2010-01-26 17:19:26 ----

I'm seeing a couple of errors in my error log coming from opac-search.pl:

opac-search.pl: Use of uninitialized value $borrowernumber in string eq at /home/oleonard/kohaclone/opac/opac-search.pl line 465.

...referring to:

  if ($borrowernumber eq '') {

(error occurs if the user is not logged in). And:

opac-search.pl: Use of uninitialized value in string eq at /home/oleonard/kohaclone/opac/opac-search.pl line 468.

...referring to:

if ($params->{'offset'} eq '') {

I notice that on line 329 a variable $offset is initialized like this:

my $offset = $params->{'offset'} || 0;

...which made me wonder whether the later check for $params->{'offset'} should be checking $offset instead? But it's not clear to me how that's being used.



--- Bug imported by chris@bigballofwax.co.nz 2010-05-21 01:22 UTC  ---

This bug was previously known as _bug_ 4064 at http://bugs.koha.org/cgi-bin/bugzilla3/show_bug.cgi?id=4064

Actual time not defined. Setting to 0.0
Setting qa contact to the default for this product.
   This bug either had no qa contact or an invalid one.

Comment 1 Mark Tompsett 2012-06-30 05:52:40 UTC Comment hidden (obsolete)
Comment 2 Julian Maurice 2012-08-06 13:37:16 UTC
Created attachment 11384 [details] [review]
[SIGNED-OFF] Bug 4064 - Uninitialized variable errors in opac-search.pl Merely optimized two if statements, already fixed in master.

Simple patch, makes code clearer.
Comment 3 Jonathan Druart 2012-08-06 13:50:50 UTC
QA Comments:

Remove a muzzy comparison in opac-search.pl

Passed QA
Comment 4 Paul Poulain 2012-09-03 13:48:06 UTC
Question: why do you write "already fixed in master." in your commit comment ? the patch applies to master, and it should be pushed

so... patch pushed
Comment 5 Mark Tompsett 2012-09-03 14:30:56 UTC
When I said fixed in master, I meant there were warning messages generated in the error log file for 3.6.3, but that the ugly logic which was there solved the problem. I merely attempted to wade through the ugly logic to find a more elegant solution. Pushing it is perfectly fine with me, but it also applies to 3.6.x and 3.8.x too, which is why I have changed the version to unspecified.
Comment 6 Chris Cormack 2012-09-03 20:54:39 UTC
Patch does not apply cleanly to 3.8.x. Please submit a patch formatted against 3.8.x if you want it in there.
Comment 7 Mark Tompsett 2012-09-04 00:37:34 UTC
Created attachment 11948 [details] [review]
Optimized two if statements (3.8.x)

Thanks for letting me know, Chris. Someone added a comment which seems to have thrown the first patch off. Here is a patch which applies to 3.8.x, and hopefully 3.6.x too. If there are problems, please let me know.
Comment 8 Chris Cormack 2012-09-04 01:21:55 UTC
The 3.8.x patch applies fine, pushed, will be in 3.8.5