Use of uninitialized value in hash element at Slim/Formats/Parse.pmline 181

[Caleb Epstein]

Warning coming from slimserver when run against a directory
containing CUE files that don't actually point to audio CDs:

Use of uninitialized value in hash element at Slim/Formats/Parse.pm line 181, <GEN23> line 6.

The offending CUE file, for a VCD, looks like:

FILE "VCD.BIN" BINARY
TRACK 01 MODE2/2352
INDEX 01 00:00:00
TRACK 02 MODE2/2352
INDEX 00 00:04:00
INDEX 01 00:06:00

The problem is that $currtrack is never being initialized.
All the if statements which depend on currtrack being defined
should probably be wrapped with an outer if (defined
$currtrack) or changed to something like:

} elsif (defined $currtrack and /^\s+TITLE \"(.*)\"/) {

I'd recommend using \s+ instead of a hard-coded whitespace
string.

--
Caleb Epstein | bklyn . org | work, n.:
cae at | Brooklyn Dust | The blessed respite from screaming
bklyn dot org | Bunny Mfg. | kids and soap operas for which you
| | actually get paid.

[dean]

Somebody got a patch for me?


On Dec 1, 2003, at 1:29 PM, Caleb Epstein wrote:

>
> Warning coming from slimserver when run against a directory
> containing CUE files that don't actually point to audio CDs:
>
> Use of uninitialized value in hash element at Slim/Formats/Parse.pm
> line 181, <GEN23> line 6.
>
> The offending CUE file, for a VCD, looks like:
>
> FILE "VCD.BIN" BINARY
> TRACK 01 MODE2/2352
> INDEX 01 00:00:00
> TRACK 02 MODE2/2352
> INDEX 00 00:04:00
> INDEX 01 00:06:00
>
> The problem is that $currtrack is never being initialized.
> All the if statements which depend on currtrack being defined
> should probably be wrapped with an outer if (defined
> $currtrack) or changed to something like:
>
> } elsif (defined $currtrack and /^\s+TITLE \"(.*)\"/) {
>
> I'd recommend using \s+ instead of a hard-coded whitespace
> string.
>
> --
> Caleb Epstein | bklyn . org | work, n.:
> cae at | Brooklyn Dust | The blessed respite from
> screaming
> bklyn dot org | Bunny Mfg. | kids and soap operas for which
> you
> | | actually get paid.
>

[Caleb Epstein]

On Mon, Dec 01, 2003 at 01:42:40PM -0800, dean blackketter wrote:

> Somebody got a patch for me?

OK, I'll bite. Here's a patch which makes the CUE file
parsing case-insensitive and less strict about whitespace. It
also removes some repetitive code by unifying several tests
into a single regexp.

diff -ubr SlimServer_v2003-12-01.orig/Slim/Formats/Parse.pm SlimServer_v2003-12-01/Slim/Formats/Parse.pm
--- SlimServer_v2003-12-01.orig/Slim/Formats/Parse.pm 2003-12-01 08:01:27.000000000 -0500
+++ SlimServer_v2003-12-01/Slim/Formats/Parse.pm 2003-12-01 16:57:20.000000000 -0500
@@ -154,30 +154,24 @@
# strip whitespace from end
s/\s*$//;

- if (/^TITLE \"(.*)\"/) {
+ if (/^TITLE\s+\"(.*)\"/i) {
$album = $1;
- } elsif (/^YEAR \"(.*)\"/) {
+ } elsif (/^YEAR\s+\"(.*)\"/i) {
$year = $1;
- } elsif (/^GENRE \"(.*)\"/) {
+ } elsif (/^GENRE\s+\"(.*)\"/i) {
$genre = $1;
- } elsif (/^COMMENT \"(.*)\"/) {
+ } elsif (/^COMMENT\s+\"(.*)\"/i) {
$comment = $1;
- } elsif (/^FILE \"(.*)\"/) {
+ } elsif (/^FILE\s+\"(.*)\"/i) {
$filename = $1;
$filename = Slim::Utils::Misc::fixPath($filename, $cuedir);
- } elsif (/^ TRACK 0*(\d+) AUDIO/) {
- $currtrack = $1;
- } elsif (/^ TITLE \"(.*)\"/) {
- $tracks{$currtrack}->{'TITLE'} = $1;
- } elsif (/^ PERFORMER \"(.*)\"/) {
- $tracks{$currtrack}->{'ARTIST'} = $1;
- } elsif (/^ YEAR \"(.*)\"/) {
- $tracks{$currtrack}->{'YEAR'} = $1;
- } elsif (/^ GENRE \"(.*)\"/) {
- $tracks{$currtrack}->{'GENRE'} = $1;
- } elsif (/^ COMMENT \"(.*)\"/) {
- $tracks{$currtrack}->{'COMMENT'} = $1;
- } elsif (/^ INDEX 01 (\d+)\d+)\d+)/) {
+ } elsif (/^\s+TRACK\s+(\d+)\s+AUDIO/i) {
+ $currtrack = int ($1);
+ } elsif (defined $currtrack and
+ /^\s+(TITLE|ARTIST|YEAR|GENRE|COMMENT)\s+\"(.*)\"/i) {
+ $tracks{$currtrack}->{uc $1} = $2;
+ } elsif (defined $currtrack and
+ /^\s+INDEX\s+01\s+(\d+)\d+)\d+)/i) {
$tracks{$currtrack}->{'START'} = ($1 * 60) + $2 + ($3 / 100);
}
}

--
Caleb Epstein | bklyn . org |Water, taken in moderation cannot hurt anybody.
cae at | Brooklyn Dust | -- Mark Twain
bklyn dot org | Bunny Mfg. |

[Caleb Epstein]

On Mon, Dec 01, 2003 at 05:03:32PM -0500, Caleb Epstein wrote:

> On Mon, Dec 01, 2003 at 01:42:40PM -0800, dean blackketter wrote:
>
> > Somebody got a patch for me?
>
> OK, I'll bite. Here's a patch which makes the CUE file
> parsing case-insensitive and less strict about whitespace. It
> also removes some repetitive code by unifying several tests
> into a single regexp.

I botched that. Need to map PERFORMER -> ARTIST. Here's
another try. Full patch versus the nightly build.

diff -ubr SlimServer_v2003-12-01.orig/Slim/Formats/Parse.pm SlimServer_v2003-12-01/Slim/Formats/Parse.pm
--- SlimServer_v2003-12-01.orig/Slim/Formats/Parse.pm 2003-12-01 08:01:27.000000000 -0500
+++ SlimServer_v2003-12-01/Slim/Formats/Parse.pm 2003-12-01 17:15:19.000000000 -0500
@@ -154,30 +154,26 @@
# strip whitespace from end
s/\s*$//;

- if (/^TITLE \"(.*)\"/) {
+ if (/^TITLE\s+\"(.*)\"/i) {
$album = $1;
- } elsif (/^YEAR \"(.*)\"/) {
+ } elsif (/^YEAR\s+\"(.*)\"/i) {
$year = $1;
- } elsif (/^GENRE \"(.*)\"/) {
+ } elsif (/^GENRE\s+\"(.*)\"/i) {
$genre = $1;
- } elsif (/^COMMENT \"(.*)\"/) {
+ } elsif (/^COMMENT\s+\"(.*)\"/i) {
$comment = $1;
- } elsif (/^FILE \"(.*)\"/) {
+ } elsif (/^FILE\s+\"(.*)\"/i) {
$filename = $1;
$filename = Slim::Utils::Misc::fixPath($filename, $cuedir);
- } elsif (/^ TRACK 0*(\d+) AUDIO/) {
- $currtrack = $1;
- } elsif (/^ TITLE \"(.*)\"/) {
- $tracks{$currtrack}->{'TITLE'} = $1;
- } elsif (/^ PERFORMER \"(.*)\"/) {
+ } elsif (/^\s+TRACK\s+(\d+)\s+AUDIO/i) {
+ $currtrack = int ($1);
+ } elsif (defined $currtrack and /^\s+PERFORMER\s+\"(.*)\"/i) {
$tracks{$currtrack}->{'ARTIST'} = $1;
- } elsif (/^ YEAR \"(.*)\"/) {
- $tracks{$currtrack}->{'YEAR'} = $1;
- } elsif (/^ GENRE \"(.*)\"/) {
- $tracks{$currtrack}->{'GENRE'} = $1;
- } elsif (/^ COMMENT \"(.*)\"/) {
- $tracks{$currtrack}->{'COMMENT'} = $1;
- } elsif (/^ INDEX 01 (\d+)\d+)\d+)/) {
+ } elsif (defined $currtrack and
+ /^\s+(TITLE|YEAR|GENRE|COMMENT)\s+\"(.*)\"/i) {
+ $tracks{$currtrack}->{uc $1} = $2;
+ } elsif (defined $currtrack and
+ /^\s+INDEX\s+01\s+(\d+)\d+)\d+)/i) {
$tracks{$currtrack}->{'START'} = ($1 * 60) + $2 + ($3 / 100);
}
}

--
Caleb Epstein | bklyn . org | BOFH excuse #56:
cae at | Brooklyn Dust |
bklyn dot org | Bunny Mfg. | Electricians made popcorn in the power supply

[Caleb Epstein]

On Mon, Dec 01, 2003 at 02:09:47PM -0800, Jason Holtzapple wrote:

> I thought a properly formed cue file required 2 and 4 space indents
> for the various elements, so loose whitespace matching might not be
> appropriate. But then again, I've never been able to find a good
> cuesheet spec.

I think it depends largely on the app consuming the data as to
what formats are acceptable or not. I have had good luck with
cdrdao consuming some pretty loosely formatted CUE files and
CDRWin seems pretty robust as well.

I have a simple script that makes CUE files for burning audio
CDs from a set of WAV audio files and this format has always
worked fine:

SILENCE 00:02:00
TRACK AUDIO
FILE "file1.wav" 0
TRACK AUDIO
FILE "file2.wav" 0

etc.

I think the whitespace is optional (or at least it is for
CDRDAO). The real marker to look for is "TRACK".

--
Caleb Epstein | bklyn . org | All is well that ends well.
cae at | Brooklyn Dust | -- John Heywood
bklyn dot org | Bunny Mfg. |