Geoffrey Young wrote: [...]
right, sorry. So it should be easy to revert in the previous code, once we figure out what gets onto the stack when exit() is called (I'd guess that it'll be undef).
according to perlapi, yes: for Perl_croak count is 1 and undef is on the stack with G_EVAL.
so why not POPs and then check status_sv == &PL_undef_sv and otherwise to go as before coercing SV into IV?
I haven't really looked at it, thinking that we can just check whether the int slot is valid. So I really introduced a potential bug then if someone was returning "0" and it used to work.
yeah, it looks like "return values as strings" used to work and now they don't. but that's ok, we'll fix it :)
another reason to have a soonish release of 1.99_11 (the other is a bug in filters insertion (connection filter being inserted into the request filter queue)
Also for some reason I wasn't able to even reproduce the undef warning in the test suite, but easily reproduced outside of the test suite.
test suites are good for excercising functionality and some error conditions, but not always for reproducing real-world issues like that, unfortunately.
Frankly I don't think this applies to the undef-warn-on-exit problem, it's reproducable in a trivial cgi script. It must be something else in the test suite that masks the problem. I might take another look at it later on. As we mess with callback return statuses, it'd be nice to have it covered in the test suite.
I think a plan coercing into IV as before will do just as well.
well, I'd like to add the integer check if we coerce the PV into an int, just to be safe - that's probably the (very) rare case, but I'd like to be able to say _something_ about it in the logs.
Checking the first char with isdigit should probably be good enough (though if you do that we need to use a crossplatform version of it, e.g. apr_isdigit)
just because perl uses 1 for true, and things like print() and other functions return 1.
but yes, we can't protect against everything. I was just trying to make it a bit more obvious if users make an obvious mistake.
so we are guessing again?
sorta. with the patch I submitted, the 1 is allowed to continue on to Apache - only a warning is thrown.
Do you think people will check their logs? And if for some reason they want it to be that way, they will just get annoyed by this log. I think we should either assert if we think it's a bug or stay quiet if we tolerate it, having a tracing tell us would be a good idea though if we get a bug report.
I somehow feel that this will be a common mistake (it showed up in the test suite, in fact) that will be difficult to debug - when a return value is forgotten, the server merely sends a 500 with no errors logged at all.
what if the return value happens to be an eval of some code which returns a valid status code, it's really guessing here...
but I'm not going to put up a fuss - I was just trying to anticipate user problems, which isn't always a good idea :)
Could be YAGNI as well.
at any rate, I'll come up with another patch tomorrow (if I have time - apachecon slides are looming). however, after this discussion, I don't think it will be too much different - just the grok_integer and special treatment of 1 issues, right?
If you really insist. I'm +1 on fixing the bug introduced by exit() fix and keeping the rest as before the s/POPi/POPs/ change (quietly coercing defined SVs to IV with any additional checks).
__________________________________________________________________ Stas Bekman JAm_pH ------> Just Another mod_perl Hacker http://stason.org/ mod_perl Guide ---> http://perl.apache.org mailto:[EMAIL PROTECTED] http://use.perl.org http://apacheweek.com http://modperlbook.org http://apache.org http://ticketmaster.com
--------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
