Bug 3153 – Incorrect error handling in Audio::FLAC::Header

Bug 3153 - Incorrect error handling in Audio::FLAC::Header

Summary:

Incorrect error handling in Audio::FLAC::Header

Status:	CLOSED FIXED

Product:	Logitech Media Server
Classification:	Unclassified
Component:	Formats
Version:	6.2.1
Platform:	PC Windows XP

Importance:	P2 minor (vote)
Target Milestone:	---
Assigned To:	Dan Sully

URL:
Keywords:

Depends on:
Blocks:
	Show dependency tree / graph

Reported:	2006-03-14 17:55 UTC by Tom Permutt
Modified:	2008-09-15 14:38 UTC (History)
CC List:	0 users

See Also:
Category:	---

Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.

Description Tom Permutt 2006-03-14 17:55:56 UTC

# $Id: Header.pm 4044 2005-08-24 19:17:11Z dsully $

At several places in the constructor method, you check for various errors, warn about them, and return $self anyway.  Shouldn't it return undef?  This is what seems to be expected by Slim::Formats::FLAC, at least:

my $flac = Audio::FLAC::Header->new($file) || do {
		errorMsg("Couldn't open file: [$file] for reading: $!\n");
		return;

This does not have the presumably desired effect of failing when "new" fails.

I'm a Perl novice; sorry if I've been a blockhead.  If it's not a bug, how do I check whether $flac references the header of a valid file or not?

Comment 1 Dan Sully 2006-04-23 13:48:18 UTC

Fixed in change 7077

Comment 2 Tom Permutt 2006-05-03 18:32:19 UTC

Still not right, though, I think.  This branch returns a hash with more than zero keys, but not a usable one:


	# make sure dos-type systems can handle it...
	binmode FILE;

	$self->{'fileSize'}   = -s $file;
	$self->{'filename'}   = $file;
	$self->{'fileHandle'} = \*FILE;

	# Initialize FLAC analysis
	$errflag = $self->_init();
	if ($errflag < 0) {
		warn "[$file] does not appear to be a FLAC file!";
		close FILE;
		undef $self->{'fileHandle'};
		return $self;
	};

Comment 3 Dan Sully 2006-05-07 14:22:38 UTC

Ok - I changed it to return undef - change 7332

Comment 4 Chris Owens 2006-06-27 14:22:06 UTC

This bug fix is now part of a released version, and so has been marked closed. If you are still experiencing this problem, please reopen the bug.