Geoffrey Young wrote:

is it possible that that later on we will have callbacks that return a string, which is a valid thing? Aren't we guessing here again?


well, I suppose DIR_MAGIC_TYPE and DECLINE_CMD are both strings, but they managed to (somehow) work before.

I suppose, that's because we never return these?


other than that, if we declare that current_callback is for calling Perl handlers (as opposed to random Perl XSUBs) then I think it's an ok assumtion. in my own XS routines that's all that I have ever wanted to do with it.

after all, the return value of modperl_callback itself is int, so it makes sense that we only accept integers back from Perl. (maybe :)


That's why I had the idea of doing a specific handling of return code, by the callback type where we know exactly what to expect. so if it's a default_port callback we know that we need to force pv->iv,


well, don't confuse the issue. this has nothing to do with default_port. the pv -> iv thing is a problem for any handler (as my tests show). I think the real culprit was the last change from POPi to POPs - maybe POPi was already doing sv_2iv conversions behind the scenes.

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). 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.


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.

but we don't need to run this code for all other handlers. If we prepare an array of supported callbacks with integer entries, a simple switch with a logic specific to each callback will be a much cleaner solution, IMHO.


again, that depends on whether modperl_callback is exclusively for Perl handlers. if so, we need to decide whether we will ever need to accept a string from them. personally, I don't see how, since it's either OK, DECLINED, DONE, or HTTP_* (or some later integer constants in 2.1).

can you think of situation where something other than an integer would be returned?

not with the currently supported callbacks, but as you seem to plan to embrace all available hooks, I'm not familiar with all of them to tell ;)


as for the rest, the return status of the callback is passed directly back to apache. the only exception are handlers with no return status, such as ModPerl::Util::exit, which gets translated as OK.



so we used to have all callbacks returning status (besides the exit() hack, which could probably be workarounded to return a special status as well), but now we get callbacks returning potentially anything.


with '9bar' yes. is there a C routine to check whether a string is a valid integer? if so, we could use it and throw 500.

Looks like Perl_grok_number is what you are after. Other than that isdigit(3) works on single chars.


But doing this in a general case would be expensive. I think a plan coercing into IV as before will do just as well. I suppose that it already uses grok_number internally.

but outside of strings, the intent (that I thought we had +1'd) was to allow handlers to return their own status and not superimpose HTTP status codes over it (which apache does anyway for HTTP).


+ /* XXX special error logging for possible Perl gotcha */
+ if (status == 1) {
+ ap_log_error(APLOG_MARK, APLOG_ERR, 0, s,
+ "Perl callback returned 1 - did you forget to return Apache::OK?\n");
+ }



why 1? it can be any value from the last evaluation if there is no explicit return.


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?



__________________________________________________________________ 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]



Reply via email to