Bug #33764 for DBD-CSV: $! is not an indicator of failure

Sun Mar 02 19:59:08 2008 IKEGAMI [...] cpan.org - Ticket created

Subject:

$! is not an indicator of failure

DBD::CSV doesn't use the documented method of determining whether Text::CSV reached the end of file or not, resulting in failures when Text::CSV_PP is used instead of Text::CSV_XS. die "Error while reading file " . $self->{'file'} . ": $!" if $!; should be die "Error while reading file " . $self->{'file'} . ": $!" if !$csv->eof();

Sun Mar 02 20:35:36 2008 jeff [...] vpservices.com - Correspondence added

CC:	mailto:IKEGAMI [...] cpan.org
Subject:	Re: [rt.cpan.org #33764] $! is not an indicator of failure
Date:	Mon, 03 Mar 2008 01:35:10 +0000
To:	bug-DBD-CSV [...] rt.cpan.org
From:	Jeff Zucker <jeff [...] vpservices.com>

If Text::CSV_PP behaves differently than Text::CSV_XS, that seems like a bug in Text:CSV_PP, why do you see it as a bug in the upstream modules? -- Jeff ikegami via RT wrote: Show quoted text

> Sun Mar 02 19:59:08 2008: Request 33764 was acted upon. > Transaction: Ticket created by ikegami > Queue: DBD-CSV > Subject: $! is not an indicator of failure > Broken in: 0.22 > Severity: Important > Owner: Nobody > Requestors: IKEGAMI@cpan.org > Status: new > Ticket <URL: http://rt.cpan.org/Ticket/Display.html?id=33764 > > > > DBD::CSV doesn't use the documented method of determining whether > Text::CSV reached the end of file or not, resulting in failures when > Text::CSV_PP is used instead of Text::CSV_XS. > > die "Error while reading file " . $self->{'file'} . ": $!" if $!; > should be > die "Error while reading file " . $self->{'file'} . ": $!" if !$csv->eof(); > > > >

Sun Mar 02 20:35:37 2008 The RT System itself - Status changed from 'new' to 'open'

Mon Mar 03 01:13:42 2008 ikegami [...] adaelis.com - Correspondence added

CC:	bug-DBD-CSV [...] rt.cpan.org
Subject:	Re: [rt.cpan.org #33764] $! is not an indicator of failure
Date:	Mon, 3 Mar 2008 01:13:29 -0500
To:	"Jeff Zucker" <jeff [...] vpservices.com>
From:	"Eric Brine" <ikegami [...] adaelis.com>

On Sun, Mar 2, 2008 at 8:35 PM, Jeff Zucker <jeff@vpservices.com> wrote: Show quoted text

> If Text::CSV_PP behaves differently than Text::CSV_XS, that seems like a > bug in Text:CSV_PP, why do you see it as a bug in the upstream modules? >

They behave the same when using the documented interface. Only when you make assumptions about their implementations do they differ, and obviously the implementations are different. The problem is that DBD::CSV relies on some undocumented side-effect of Text::CSV_XS's implementation. -> You assume $! will continue to be set in future versions of the CSV parser class. -> You assume $! is currently always set when the CSV parser class encounters an error. Not only that, but that use of $! is buggy for much more basic reasons: -> You falsely assume zero is not a valid error code for $!. -> You falsely assume $! remains unchanged when no I/O error occur. perlvar clearly and emphatically states $! is meaningless (i.e. could be anything) in that situation. The documentation clearly favours the use of eof, so why use $!? If parse () or getline () was used with an IO stream, this method will return true (1) if the last call hit end of file, otherwise it will return false (''). This is useful to see the difference between a failure and end of file.") ELB/ikegami

Mon Aug 11 13:49:59 2008 HMBRAND [...] cpan.org - Correspondence added

On Mon Mar 03 01:13:42 2008, ikegami@adaelis.com wrote: Show quoted text

> On Sun, Mar 2, 2008 at 8:35 PM, Jeff Zucker <jeff@vpservices.com>

wrote: Show quoted text

>

> > If Text::CSV_PP behaves differently than Text::CSV_XS, that seems

like a Show quoted text

> > bug in Text:CSV_PP, why do you see it as a bug in the upstream

modules? Show quoted text

> >

> > They behave the same when using the documented interface. Only when

you make Show quoted text

> assumptions about their implementations do they differ, and obviously

the Show quoted text

> implementations are different. > > The problem is that DBD::CSV relies on some undocumented side-effect

of Show quoted text

> Text::CSV_XS's implementation. > > -> You assume $! will continue to be set in future versions of the CSV > parser class. > -> You assume $! is currently always set when the CSV parser class > encounters an error.

If you rely on $! to be the error from Text::CSV_??, don't. We now have $csv->error_diag (), which is much more versatile. I tested DBD::CSV today for my talk and missed error_diag () output deerly.

Wed Jun 10 02:02:36 2009 HMBRAND [...] cpan.org - Fixed in 0.24_01 added

Wed Jun 10 02:02:36 2009 HMBRAND [...] cpan.org - Status changed from 'open' to 'resolved'