Mailing List Archive: Catalyst: Dev

Annoying undef warnings from CP::Static::Simple

Index | Next | Previous | View Threaded

autarch at urth

May 30, 2008, 4:35 PM

Post #1 of 6 (1799 views)
Permalink

Annoying undef warnings from CP::Static::Simple

I'm getting a bunch of warnings from requests for static content that look
like this:

Use of uninitialized value in length at
/usr/local/share/perl/5.8.8/Catalyst/Dispatcher.pm line 287.

The problem is that Catalyst::Dispatcher expects $c->req->match to be
defined, but it isn't when CPSS does the dispatching.

I'm not sure where this should be fixed. Should $c->req->match always be
set? Or should CD just not assume it is defined?

-dave

/*==========================
VegGuide.Org
Your guide to all that's veg
==========================*/

_______________________________________________
Catalyst-dev mailing list
Catalyst-dev [at] lists
http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst-dev

dbix-class at trout

May 31, 2008, 1:00 AM

Post #2 of 6 (1729 views)
Permalink

Re: Annoying undef warnings from CP::Static::Simple [In reply to]

On Fri, May 30, 2008 at 06:35:11PM -0500, Dave Rolsky wrote:
> I'm getting a bunch of warnings from requests for static content that look
> like this:
>
> Use of uninitialized value in length at
> /usr/local/share/perl/5.8.8/Catalyst/Dispatcher.pm line 287.
>
> The problem is that Catalyst::Dispatcher expects $c->req->match to be
> defined, but it isn't when CPSS does the dispatching.
>
> I'm not sure where this should be fixed. Should $c->req->match always be
> set? Or should CD just not assume it is defined?

There's no $c->action at all when CPSS fires so I don't think I can really
see $c->req->match as making a lot of sense in that case.

This has already been reported via rt.cpan but sans test; if you can whip one
up there's no reason we can't add a defined() check.

--
Matt S Trout Need help with your Catalyst or DBIx::Class project?
Technical Director http://www.shadowcat.co.uk/catalyst/
Shadowcat Systems Ltd. Want a managed development or deployment platform?
http://chainsawblues.vox.com/ http://www.shadowcat.co.uk/servers/

_______________________________________________
Catalyst-dev mailing list
Catalyst-dev [at] lists
http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst-dev

autarch at urth

May 31, 2008, 7:30 AM

Post #3 of 6 (1718 views)
Permalink

Re: Annoying undef warnings from CP::Static::Simple [In reply to]

On Sat, 31 May 2008, Matt S Trout wrote:

> On Fri, May 30, 2008 at 06:35:11PM -0500, Dave Rolsky wrote:
>> I'm getting a bunch of warnings from requests for static content that look
>> like this:
>>
>> Use of uninitialized value in length at
>> /usr/local/share/perl/5.8.8/Catalyst/Dispatcher.pm line 287.
>>
>> The problem is that Catalyst::Dispatcher expects $c->req->match to be
>> defined, but it isn't when CPSS does the dispatching.
>>
>> I'm not sure where this should be fixed. Should $c->req->match always be
>> set? Or should CD just not assume it is defined?
>
> There's no $c->action at all when CPSS fires so I don't think I can really
> see $c->req->match as making a lot of sense in that case.
>
> This has already been reported via rt.cpan but sans test; if you can whip one
> up there's no reason we can't add a defined() check.

I've attached a new test and a patch to make it pass.

As an aside, I realized that part of the reason this was never fixed is
that the core Catalyst code does not enable "use warnings" in the modules.
However, if you run under the -w flag like the standalone server does,
then you get all the warnings from everywhere.

I think it'd be a good idea to turn on warnings in the core modules and
then shut them up. Are such patches welcome?

-dave

/*==========================
VegGuide.Org
Your guide to all that's veg
==========================*/

Attachments:

dispatcher.patch (2.08 KB)

jon at jrock

May 31, 2008, 11:23 AM

Post #4 of 6 (1707 views)
Permalink

Re: Annoying undef warnings from CP::Static::Simple [In reply to]

* On Sat, May 31 2008, Dave Rolsky wrote:
> On Sat, 31 May 2008, Matt S Trout wrote:
>
>> On Fri, May 30, 2008 at 06:35:11PM -0500, Dave Rolsky wrote:
>>> I'm getting a bunch of warnings from requests for static content that look
>>> like this:
>>>
>>> Use of uninitialized value in length at
>>> /usr/local/share/perl/5.8.8/Catalyst/Dispatcher.pm line 287.
>>>
>>> The problem is that Catalyst::Dispatcher expects $c->req->match to be
>>> defined, but it isn't when CPSS does the dispatching.
>>>
>>> I'm not sure where this should be fixed. Should $c->req->match always be
>>> set? Or should CD just not assume it is defined?
>>
>> There's no $c->action at all when CPSS fires so I don't think I can really
>> see $c->req->match as making a lot of sense in that case.
>>
>> This has already been reported via rt.cpan but sans test; if you can whip one
>> up there's no reason we can't add a defined() check.
>
> I've attached a new test and a patch to make it pass.
>
> As an aside, I realized that part of the reason this was never fixed
> is that the core Catalyst code does not enable "use warnings" in the
> modules. However, if you run under the -w flag like the standalone
> server does, then you get all the warnings from everywhere.
>
> I think it'd be a good idea to turn on warnings in the core modules
> and then shut them up. Are such patches welcome?

Yes. (This is a good time to try git-svn, btw. Make a branch, make one
commit for each fix, then tell git to dump the whole set onto
the mailing list. Very convenient.)

Just for the record, sometimes I think turning off warnings is better
than "fixing" them, for example, I think:

{ no warnings;
if($foo eq 'bar'){ ... }
}

is easier to read than:

if($foo && $foo eq 'bar'){ ... }

The second adds code that serves absolutely no algorithmic purpose, and
I'm against that.

Regards,
Jonathan Rockway

--
print just => another => perl => hacker => if $,=$"

_______________________________________________
Catalyst-dev mailing list
Catalyst-dev [at] lists
http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst-dev

pagaltzis at gmx

May 31, 2008, 2:59 PM

Post #5 of 6 (1723 views)
Permalink

Re: Annoying undef warnings from CP::Static::Simple [In reply to]

* Jonathan Rockway <jon [at] jrock> [2008-05-31 20:30]:
> Just for the record, sometimes I think turning off warnings is
> better than "fixing" them, for example, I think:
>
> { no warnings;
> if($foo eq 'bar'){ ... }
> }
>
> is easier to read than:
>
> if($foo && $foo eq 'bar'){ ... }
>
> The second adds code that serves absolutely no algorithmic
> purpose, and I'm against that.

And the first one… doesn’t? An extra statement and an increase in
the level of indentation is not what I consider “no extra code.”

Not only that, but what you write will disable *all* warnings for
a possibly long stretch of code. A less sloppy way is to at least
say `no warnings 'uninitialized';`, but the right of doing this,
which also avoids the extra indentation, would be thus:

if ( do { no warnings; $foo eq 'bar' } ) {
# ...
}

Except… this is harder to read than merely adding an extra truth
test. And it is often the case that proper local disabling of
warnings is more effort than just writing extra tests to avoid
triggering the warning(s).

But if you’re not going to disable them properly, then admit to
yourself that you don’t care and say so: put a `no warnings` at
the top of a scope or maybe at the top of the entire file and
move on. I have no problem with that; almost all my Perl files
have `no warnings qw( once qw );` at the top.

But pick either fish or fowl. Half-arsing it gives you all the
drawbacks of both approaches without any of the benefits.

Regards,
--
Aristotle Pagaltzis // <http://plasmasturm.org/>

_______________________________________________
Catalyst-dev mailing list
Catalyst-dev [at] lists
http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst-dev

dbix-class at trout

Jun 1, 2008, 12:01 AM

Post #6 of 6 (1709 views)
Permalink

Re: Annoying undef warnings from CP::Static::Simple [In reply to]

On Sat, May 31, 2008 at 09:30:14AM -0500, Dave Rolsky wrote:
> On Sat, 31 May 2008, Matt S Trout wrote:
>
> >On Fri, May 30, 2008 at 06:35:11PM -0500, Dave Rolsky wrote:
> >>I'm getting a bunch of warnings from requests for static content that look
> >>like this:
> >>
> >> Use of uninitialized value in length at
> >> /usr/local/share/perl/5.8.8/Catalyst/Dispatcher.pm line 287.
> >>
> >>The problem is that Catalyst::Dispatcher expects $c->req->match to be
> >>defined, but it isn't when CPSS does the dispatching.
> >>
> >>I'm not sure where this should be fixed. Should $c->req->match always be
> >>set? Or should CD just not assume it is defined?
> >
> >There's no $c->action at all when CPSS fires so I don't think I can really
> >see $c->req->match as making a lot of sense in that case.
> >
> >This has already been reported via rt.cpan but sans test; if you can whip
> >one
> >up there's no reason we can't add a defined() check.
>
> I've attached a new test and a patch to make it pass.
>
> As an aside, I realized that part of the reason this was never fixed is
> that the core Catalyst code does not enable "use warnings" in the modules.
> However, if you run under the -w flag like the standalone server does,
> then you get all the warnings from everywhere.
>
> I think it'd be a good idea to turn on warnings in the core modules and
> then shut them up. Are such patches welcome?

Yes, against the moose branch off 5.80 - 'use Moose' already turns warnings
on so it's half-way there already and the timeframe for that branch gives us
a reasonable chance of not inflicting a bunch of warnings on users that are
realistically our fault.

--
Matt S Trout Need help with your Catalyst or DBIx::Class project?
Technical Director http://www.shadowcat.co.uk/catalyst/
Shadowcat Systems Ltd. Want a managed development or deployment platform?
http://chainsawblues.vox.com/ http://www.shadowcat.co.uk/servers/

_______________________________________________
Catalyst-dev mailing list
Catalyst-dev [at] lists
http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst-dev

Index | Next | Previous | View Threaded

Interested in having your list archived? Contact Gossamer Threads