You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@perl.apache.org by Stas Bekman <st...@stason.org> on 2004/04/08 07:35:46 UTC
[mp2 patch] $socket_opt_(get|set) API and test
todays Joe patch triggered my interest to further test the socket set/get
options, just to discover that opt_get doesn't work at all. So I made it
working and made both APIs perlish, croacking in case of the failure instead
of returning status. Sounds good?
Index: t/protocol/TestProtocol/echo.pm
===================================================================
RCS file: /home/cvs/modperl-2.0/t/protocol/TestProtocol/echo.pm,v
retrieving revision 1.4
diff -u -r1.4 echo.pm
--- t/protocol/TestProtocol/echo.pm 8 Apr 2004 00:11:25 -0000 1.4
+++ t/protocol/TestProtocol/echo.pm 8 Apr 2004 05:32:33 -0000
@@ -15,13 +15,19 @@
my Apache::Connection $c = shift;
my APR::Socket $socket = $c->client_socket;
- my $buff;
-
# make sure the socket is in the blocking mode for recv().
# on some platforms (e.g. OSX/Solaris) httpd hands us a
# non-blocking socket
- $socket->opt_set(APR::SO_NONBLOCK, 0);
+ my $nonblocking = $socket->opt_get(APR::SO_NONBLOCK);
+ if ($nonblocking) {
+ my $prev_value = $socket->opt_set(APR::SO_NONBLOCK => 0);
+ # test that we really are in the non-blocking mode
+ $nonblocking = $socket->opt_get(APR::SO_NONBLOCK);
+ die "failed to set non-blocking mode" if $nonblocking;
+ }
+
+ my $buff;
for (;;) {
my($rlen, $wlen);
$rlen = BUFF_LEN;
Index: xs/APR/Socket/APR__Socket.h
===================================================================
RCS file: /home/cvs/modperl-2.0/xs/APR/Socket/APR__Socket.h,v
retrieving revision 1.4
diff -u -r1.4 APR__Socket.h
--- xs/APR/Socket/APR__Socket.h 4 Mar 2004 06:01:10 -0000 1.4
+++ xs/APR/Socket/APR__Socket.h 8 Apr 2004 05:32:34 -0000
@@ -64,3 +64,20 @@
return t;
}
+static MP_INLINE apr_int32_t
+mpxs_APR__Socket_opt_get(pTHX_ apr_socket_t *socket, apr_int32_t opt)
+{
+ apr_int32_t val;
+ MP_FAILURE_CROAK(apr_socket_opt_get(socket, opt, &val));
+ return val;
+}
+
+static MP_INLINE apr_int32_t
+mpxs_APR__Socket_opt_set(pTHX_ apr_socket_t *socket, apr_int32_t opt,
+ apr_int32_t val)
+{
+ apr_int32_t oldval;
+ MP_FAILURE_CROAK(apr_socket_opt_get(socket, opt, &oldval));
+ MP_FAILURE_CROAK(apr_socket_opt_set(socket, opt, val));
+ return oldval;
+}
Index: xs/maps/apr_functions.map
===================================================================
RCS file: /home/cvs/modperl-2.0/xs/maps/apr_functions.map,v
retrieving revision 1.70
diff -u -r1.70 apr_functions.map
--- xs/maps/apr_functions.map 31 Jan 2004 10:06:59 -0000 1.70
+++ xs/maps/apr_functions.map 8 Apr 2004 05:32:35 -0000
@@ -58,8 +58,10 @@
!apr_socket_addr_get
!apr_socket_data_get
!apr_socket_data_set
- apr_socket_opt_get
- apr_socket_opt_set
+-apr_socket_opt_get
+-apr_socket_opt_set
+ mpxs_APR__Socket_opt_get
+ mpxs_APR__Socket_opt_set
apr_socket_timeout_get | mpxs_ | ...
apr_socket_timeout_set
-apr_socket_sendfile
Index: xs/tables/current/ModPerl/FunctionTable.pm
===================================================================
RCS file: /home/cvs/modperl-2.0/xs/tables/current/ModPerl/FunctionTable.pm,v
retrieving revision 1.149
diff -u -r1.149 FunctionTable.pm
--- xs/tables/current/ModPerl/FunctionTable.pm 2 Apr 2004 02:17:46 -0000 1.149
+++ xs/tables/current/ModPerl/FunctionTable.pm 8 Apr 2004 05:32:35 -0000
@@ -6829,6 +6829,54 @@
]
},
{
+ 'return_type' => 'apr_int32_t',
+ 'name' => 'mpxs_APR__Socket_opt_get',
+ 'attr' => [
+ 'static',
+ '__inline__'
+ ],
+ 'args' => [
+ {
+ 'type' => 'PerlInterpreter *',
+ 'name' => 'my_perl'
+ },
+ {
+ 'type' => 'apr_socket_t *',
+ 'name' => 'socket'
+ },
+ {
+ 'type' => 'apr_int32_t',
+ 'name' => 'opt'
+ },
+ ]
+ },
+ {
+ 'return_type' => 'apr_int32_t',
+ 'name' => 'mpxs_APR__Socket_opt_set',
+ 'attr' => [
+ 'static',
+ '__inline__'
+ ],
+ 'args' => [
+ {
+ 'type' => 'PerlInterpreter *',
+ 'name' => 'my_perl'
+ },
+ {
+ 'type' => 'apr_socket_t *',
+ 'name' => 'socket'
+ },
+ {
+ 'type' => 'apr_int32_t',
+ 'name' => 'opt'
+ },
+ {
+ 'type' => 'apr_int32_t',
+ 'name' => 'val'
+ },
+ ]
+ },
+ {
'return_type' => '',
'name' => 'mpxs_apr_sockaddr_ip_get',
'args' => [
--
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
[mp2 patch take2] $socket_opt_(get|set) API and test
Posted by Stas Bekman <st...@stason.org>.
OK, here is a new version which returns undef on failure and sets $APR::err to
the error message:
Index: src/modules/perl/modperl_util.h
===================================================================
RCS file: /home/cvs/modperl-2.0/src/modules/perl/modperl_util.h,v
retrieving revision 1.53
diff -u -r1.53 modperl_util.h
--- src/modules/perl/modperl_util.h 2 Apr 2004 02:17:45 -0000 1.53
+++ src/modules/perl/modperl_util.h 8 Apr 2004 19:21:46 -0000
@@ -72,6 +72,18 @@
} \
} while (0)
+
+/* runs a given code and if failed sets $APR::err to the error message
+ * and returns &PL_sv_undef */
+#define MP_APR_RETURN_ON_FAILURE(rc_run) do { \
+ apr_status_t rc = (rc_run); \
+ if (rc != APR_SUCCESS) { \
+ GV *gv = gv_fetchpv("APR::err", GV_ADDMULTI, SVt_PV); \
+ sv_setpv(GvSV(gv), modperl_apr_strerror(rc)); \
+ return &PL_sv_undef; \
+ } \
+ } while (0)
+
/* check whether the response phase has been initialized already */
#define MP_CHECK_WBUCKET_INIT(func) \
if (!rcfg->wbucket) { \
Index: t/protocol/TestProtocol/echo.pm
===================================================================
RCS file: /home/cvs/modperl-2.0/t/protocol/TestProtocol/echo.pm,v
retrieving revision 1.4
diff -u -r1.4 echo.pm
--- t/protocol/TestProtocol/echo.pm 8 Apr 2004 00:11:25 -0000 1.4
+++ t/protocol/TestProtocol/echo.pm 8 Apr 2004 19:21:47 -0000
@@ -15,13 +15,21 @@
my Apache::Connection $c = shift;
my APR::Socket $socket = $c->client_socket;
- my $buff;
-
# make sure the socket is in the blocking mode for recv().
# on some platforms (e.g. OSX/Solaris) httpd hands us a
# non-blocking socket
- $socket->opt_set(APR::SO_NONBLOCK, 0);
+ my $nonblocking = $socket->opt_get(APR::SO_NONBLOCK);
+ die "failed to \$socket->opt_get: $ARP::err" unless defined $nonblocking;
+ if ($nonblocking) {
+ my $prev_value = $socket->opt_set(APR::SO_NONBLOCK => 0);
+ die "failed to \$socket->opt_get: $ARP::err" unless defined $prev_value;
+ # test that we really are in the non-blocking mode
+ $nonblocking = $socket->opt_get(APR::SO_NONBLOCK);
+ die "failed to set non-blocking mode" if $nonblocking;
+ }
+
+ my $buff;
for (;;) {
my($rlen, $wlen);
$rlen = BUFF_LEN;
Index: xs/APR/Socket/APR__Socket.h
===================================================================
RCS file: /home/cvs/modperl-2.0/xs/APR/Socket/APR__Socket.h,v
retrieving revision 1.4
diff -u -r1.4 APR__Socket.h
--- xs/APR/Socket/APR__Socket.h 4 Mar 2004 06:01:10 -0000 1.4
+++ xs/APR/Socket/APR__Socket.h 8 Apr 2004 19:21:48 -0000
@@ -64,3 +64,20 @@
return t;
}
+static MP_INLINE SV *
+mpxs_APR__Socket_opt_get(pTHX_ apr_socket_t *socket, apr_int32_t opt)
+{
+ apr_int32_t val;
+ MP_APR_RETURN_ON_FAILURE(apr_socket_opt_get(socket, opt, &val));
+ return newSViv(val);
+}
+
+static MP_INLINE SV *
+mpxs_APR__Socket_opt_set(pTHX_ apr_socket_t *socket, apr_int32_t opt,
+ apr_int32_t val)
+{
+ apr_int32_t oldval;
+ MP_APR_RETURN_ON_FAILURE(apr_socket_opt_get(socket, opt, &oldval));
+ MP_APR_RETURN_ON_FAILURE(apr_socket_opt_set(socket, opt, val));
+ return newSViv(oldval);
+}
Index: xs/maps/apr_functions.map
===================================================================
RCS file: /home/cvs/modperl-2.0/xs/maps/apr_functions.map,v
retrieving revision 1.70
diff -u -r1.70 apr_functions.map
--- xs/maps/apr_functions.map 31 Jan 2004 10:06:59 -0000 1.70
+++ xs/maps/apr_functions.map 8 Apr 2004 19:21:48 -0000
@@ -58,8 +58,10 @@
!apr_socket_addr_get
!apr_socket_data_get
!apr_socket_data_set
- apr_socket_opt_get
- apr_socket_opt_set
+-apr_socket_opt_get
+-apr_socket_opt_set
+ mpxs_APR__Socket_opt_get
+ mpxs_APR__Socket_opt_set
apr_socket_timeout_get | mpxs_ | ...
apr_socket_timeout_set
-apr_socket_sendfile
Index: xs/tables/current/ModPerl/FunctionTable.pm
===================================================================
RCS file: /home/cvs/modperl-2.0/xs/tables/current/ModPerl/FunctionTable.pm,v
retrieving revision 1.149
diff -u -r1.149 FunctionTable.pm
--- xs/tables/current/ModPerl/FunctionTable.pm 2 Apr 2004 02:17:46 -0000 1.149
+++ xs/tables/current/ModPerl/FunctionTable.pm 8 Apr 2004 19:21:49 -0000
@@ -6829,6 +6829,54 @@
]
},
{
+ 'return_type' => 'SV *',
+ 'name' => 'mpxs_APR__Socket_opt_get',
+ 'attr' => [
+ 'static',
+ '__inline__'
+ ],
+ 'args' => [
+ {
+ 'type' => 'PerlInterpreter *',
+ 'name' => 'my_perl'
+ },
+ {
+ 'type' => 'apr_socket_t *',
+ 'name' => 'socket'
+ },
+ {
+ 'type' => 'apr_int32_t',
+ 'name' => 'opt'
+ },
+ ]
+ },
+ {
+ 'return_type' => 'SV *',
+ 'name' => 'mpxs_APR__Socket_opt_set',
+ 'attr' => [
+ 'static',
+ '__inline__'
+ ],
+ 'args' => [
+ {
+ 'type' => 'PerlInterpreter *',
+ 'name' => 'my_perl'
+ },
+ {
+ 'type' => 'apr_socket_t *',
+ 'name' => 'socket'
+ },
+ {
+ 'type' => 'apr_int32_t',
+ 'name' => 'opt'
+ },
+ {
+ 'type' => 'apr_int32_t',
+ 'name' => 'val'
+ },
+ ]
+ },
+ {
'return_type' => '',
'name' => 'mpxs_apr_sockaddr_ip_get',
'args' => [
--
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [mp2 patch] $socket_opt_(get|set) API and test
Posted by Stas Bekman <st...@stason.org>.
Geoffrey Young wrote:
>>>I don't know enough about sockets. is the 0 important? in cases like
>>>this
>>>can we return 0E0?
>>
>>
>>0 is very important, in the particular test in my patch. Using 0E0? But
>>then we have an inconsistent API, some places we return undef other 0E0?
>>Granted it is already inconsistent, but the idea is to fix it.
>
>
> I didn't mean 0E0 instead of undef. I meant 0E0 for 0 - zero but true.
> that would make only errors return false (undef) while valid values
> (including zero) would all evaluate to true.
but you won't be able to replace a conditional which doesn't explicitly check
for zero.
my $nonblocking = $socket->opt_get(APR::SO_NONBLOCK);
if ($nonblocking) {
it will be even more confusing and error prone.
> I guess I was asking if the numeric '0' was important in such a way that
> perl's treatment of 0E0 as 0 would not be sufficient, such as when passing
> the result of one call (0E0) to another APR method that expected the true
> integer 0. if perl takes care of the translation then this seems like a
> real solution.
In this case it's probably going to be used in conditionals, so
it is not any different from returning undef, 0, 1. Here you'd code:
my $val = foo();
die "whatever " unless defined $val;
if ($val == 0) {
...
}
elsif ($val > 0) {
...
}
This is so much macaroni, compared to:
if (foo()) { # non-zero
...
}
else { # zero
...
}
of course adding eval it looks similar:
my $val = eval { foo() };
die "whatever " if $@;
if ($val) { # non-zero
...
}
else { # zero
...
}
If you return 0E0 a user putting it in a condition will get wrong logic.
>>>yes, affecting all interpreters for a given (virtual) server.
>>
>>
>>actually only the current interpreter, you can't affect other
>>interpreter during request time.
>
>
> if the interpreter scope is per-handler and the new PerlCroakOnFailure is
> per-directory then you'd have some modules croaking and some not in the same
> request, right?
Right, which is not good. In which case that flag should be set in the global
server structure. And it will still be broken, since one request will run the
code that will set it, but won't unset it. and another request not setting it
will be affected. It's easy with DBI, since it's all encapsulated in the
object which goes away and doesn't affect other code. Here we are in a big
mess of having too many different scopes. Not even close DWIM.
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [mp2 patch] $socket_opt_(get|set) API and test
Posted by Geoffrey Young <ge...@modperlcookbook.org>.
>> I don't know enough about sockets. is the 0 important? in cases like
>> this
>> can we return 0E0?
>
>
> 0 is very important, in the particular test in my patch. Using 0E0? But
> then we have an inconsistent API, some places we return undef other 0E0?
> Granted it is already inconsistent, but the idea is to fix it.
I didn't mean 0E0 instead of undef. I meant 0E0 for 0 - zero but true.
that would make only errors return false (undef) while valid values
(including zero) would all evaluate to true.
I guess I was asking if the numeric '0' was important in such a way that
perl's treatment of 0E0 as 0 would not be sufficient, such as when passing
the result of one call (0E0) to another APR method that expected the true
integer 0. if perl takes care of the translation then this seems like a
real solution.
>> yes, affecting all interpreters for a given (virtual) server.
>
>
> actually only the current interpreter, you can't affect other
> interpreter during request time.
if the interpreter scope is per-handler and the new PerlCroakOnFailure is
per-directory then you'd have some modules croaking and some not in the same
request, right?
--Geoff
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [mp2 patch] $socket_opt_(get|set) API and test
Posted by Stas Bekman <st...@stason.org>.
Geoffrey Young wrote:
>>We could have a directive which could control the overall behavior:
>>whether to croak or not (in addition to setting ::err vars). So one
>>could turn it on to get the server pinpoint problems if they didn't code
>>thing properly and have misterious failures.
>
>
> yes, similar to DBI's RaiseError attribute. sounds like lots of work, though.
right. Probably not too much work if we use macros to do the job and do it
right from the very beginning. Just to move from croaks to return undef/set
error var we need to change lots of functions. So we better do that once. e.g.
we could plant a noop macro which will croak if something and then enable it
later.
>>for example when you check
>>for a blocking IO mode, you will get a false value either on failure (if
>>we return undef) or when it returns 0.
>
>
> I don't know enough about sockets. is the 0 important? in cases like this
> can we return 0E0?
0 is very important, in the particular test in my patch. Using 0E0? But then
we have an inconsistent API, some places we return undef other 0E0? Granted it
is already inconsistent, but the idea is to fix it.
>>Turning that crock on failure
>>feature on, will tell enforce croaking showing where the problem is. I
>>like that idea. Though there is a problem with it. if a developer relies
>>on the automatic croak without coding his own error checking a user may
>>not have this feature turning on and then he will get misterious
>>misbehaviors.
>
>
> yeah, that's the problem - modules will either trap their errors or not.
>
>
>>So perhaps that should be turned on/off from any place in
>>the code and not only from startup time.
>
>
> yes, affecting all interpreters for a given (virtual) server.
actually only the current interpreter, you can't affect other interpreter
during request time.
>>Implementation wise, it should
>>affect everything I believe (i.e. it's either on or off), but again this
>>may affect other people's code who did check for errors and tried to
>>take alternative solutions if something has failed, which when this mode
>>is enabled will not work as expected. So I'm not sure... May be we could
>>advertise it as a debugging tool, not to be used in CPAN modules?
>
>
> right, CPAN is the bigger issue.
>
> anyway, I dunno. something in me feels like this is probably a bad idea, or
> at least not worth the effort, but I'm not convinced either way. if you
> want to take a crack at it go for it.
It's just too big of a change to "give it a crack". I'll try to see what can
be done with the socket set/get API though.
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [mp2 patch] $socket_opt_(get|set) API and test
Posted by Geoffrey Young <ge...@modperlcookbook.org>.
> We could have a directive which could control the overall behavior:
> whether to croak or not (in addition to setting ::err vars). So one
> could turn it on to get the server pinpoint problems if they didn't code
> thing properly and have misterious failures.
yes, similar to DBI's RaiseError attribute. sounds like lots of work, though.
> for example when you check
> for a blocking IO mode, you will get a false value either on failure (if
> we return undef) or when it returns 0.
I don't know enough about sockets. is the 0 important? in cases like this
can we return 0E0?
> Turning that crock on failure
> feature on, will tell enforce croaking showing where the problem is. I
> like that idea. Though there is a problem with it. if a developer relies
> on the automatic croak without coding his own error checking a user may
> not have this feature turning on and then he will get misterious
> misbehaviors.
yeah, that's the problem - modules will either trap their errors or not.
> So perhaps that should be turned on/off from any place in
> the code and not only from startup time.
yes, affecting all interpreters for a given (virtual) server.
> Implementation wise, it should
> affect everything I believe (i.e. it's either on or off), but again this
> may affect other people's code who did check for errors and tried to
> take alternative solutions if something has failed, which when this mode
> is enabled will not work as expected. So I'm not sure... May be we could
> advertise it as a debugging tool, not to be used in CPAN modules?
right, CPAN is the bigger issue.
anyway, I dunno. something in me feels like this is probably a bad idea, or
at least not worth the effort, but I'm not convinced either way. if you
want to take a crack at it go for it.
--Geoff
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [mp2 patch] $socket_opt_(get|set) API and test
Posted by Stas Bekman <st...@stason.org>.
>>and usually if there is a socket problem, you can't do much, and
>>there is no point to eval. Most likely you will want to die.
>
>
> sure. but it's the forced die that bothers me. at least if we return undef
> they can die with their own message or throw out something other than an
> automatic 500.
right, see below
>>Second, let's say we don't croak and return undef. We need to set a
>>consistent variable with the error message. As you saw I've failed to
>>convince p5p to allow us to peruse $!. And $@ is not appropriate, we
>>have no other choce but to use some other variable. So what should it
>>be? $Apache::err and $Apache::errstr, ala $DBI::errstr. Or should there
>>be a separate error variables for APR and Apache, since APR eventually
>>could be used outside mod_perl?
>
>
> $Apache::err and $APR::err sound reasonable enough - most people familiar
> with mod_perl are familiar with DBI so it should be easy to adopt.
+1
> it's a
> shame that we can't use $!, though, since lots of the functions have the
> same name as perl built-ins.
We could have a directive which could control the overall behavior: whether to
croak or not (in addition to setting ::err vars). So one could turn it on to
get the server pinpoint problems if they didn't code thing properly and have
misterious failures. for example when you check for a blocking IO mode, you
will get a false value either on failure (if we return undef) or when it
returns 0. Turning that crock on failure feature on, will tell enforce
croaking showing where the problem is. I like that idea. Though there is a
problem with it. if a developer relies on the automatic croak without coding
his own error checking a user may not have this feature turning on and then he
will get misterious misbehaviors. So perhaps that should be turned on/off from
any place in the code and not only from startup time. Implementation wise, it
should affect everything I believe (i.e. it's either on or off), but again
this may affect other people's code who did check for errors and tried to take
alternative solutions if something has failed, which when this mode is enabled
will not work as expected. So I'm not sure... May be we could advertise it as
a debugging tool, not to be used in CPAN modules?
>>Also going back to my patch, what do you think about opt_set returning
>>an old value. I've coded explicitly to get that old value before setting
>>the new one. That sounds Perlish, but I'm not sure whether it's worth
>>the overhead.
>
>
> since you're asking, I actually prefer returning the old value on set.
Well, that's exactly what my patch does. :)
> mp1
> did that for lots of things, which made typical mp-type overrides
> (server_rec stuff, for example) able to eliminate an extra call to get()
>
> my $old = $foo->set($val); # override
> $foo->set($old); # restore
And that's exactly why I coded it that way :)
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [mp2 patch] $socket_opt_(get|set) API and test
Posted by Geoffrey Young <ge...@modperlcookbook.org>.
>> and I know we've discussed this before, but I really don't like the
>> idea of
>> croaking at request time - for configuration things it's ok, but the vast
>> majority of file IO and other operations in perl proper don't die: open,
>> read, most of the IO:: stuff, etc. in other words, people expect to
>> check
>> return values for these things instead of needing eval{} so we're
>> throwing
>> them a curve every time we croak instead.
>
>
> Agreed, but first, you probably also agree that most people won't check
> it
of course :)
> and usually if there is a socket problem, you can't do much, and
> there is no point to eval. Most likely you will want to die.
sure. but it's the forced die that bothers me. at least if we return undef
they can die with their own message or throw out something other than an
automatic 500.
>
> Second, let's say we don't croak and return undef. We need to set a
> consistent variable with the error message. As you saw I've failed to
> convince p5p to allow us to peruse $!. And $@ is not appropriate, we
> have no other choce but to use some other variable. So what should it
> be? $Apache::err and $Apache::errstr, ala $DBI::errstr. Or should there
> be a separate error variables for APR and Apache, since APR eventually
> could be used outside mod_perl?
$Apache::err and $APR::err sound reasonable enough - most people familiar
with mod_perl are familiar with DBI so it should be easy to adopt. it's a
shame that we can't use $!, though, since lots of the functions have the
same name as perl built-ins.
>
> Also going back to my patch, what do you think about opt_set returning
> an old value. I've coded explicitly to get that old value before setting
> the new one. That sounds Perlish, but I'm not sure whether it's worth
> the overhead.
since you're asking, I actually prefer returning the old value on set. mp1
did that for lots of things, which made typical mp-type overrides
(server_rec stuff, for example) able to eliminate an extra call to get()
my $old = $foo->set($val); # override
$foo->set($old); # restore
--Geoff
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [mp2 patch] $socket_opt_(get|set) API and test
Posted by Stas Bekman <st...@stason.org>.
Geoffrey Young wrote:
>
> Stas Bekman wrote:
>
>>todays Joe patch triggered my interest to further test the socket
>>set/get options, just to discover that opt_get doesn't work at all. So I
>>made it working and made both APIs perlish, croacking in case of the
>>failure instead of returning status. Sounds good?
>
>
> I like the idea of returning the value instead of the status - that's
> something that we should probably enforce more consistently than we do.
So, we can still change it. Just name the API that should be changed.
> I'm not entirely convinced that croak is a good thing. consider that perl's
> native getsockopt() and setsockopt() both return undef on error rather than die.
Neither do I.
> and I know we've discussed this before, but I really don't like the idea of
> croaking at request time - for configuration things it's ok, but the vast
> majority of file IO and other operations in perl proper don't die: open,
> read, most of the IO:: stuff, etc. in other words, people expect to check
> return values for these things instead of needing eval{} so we're throwing
> them a curve every time we croak instead.
Agreed, but first, you probably also agree that most people won't check it and
usually if there is a socket problem, you can't do much, and there is no point
to eval. Most likely you will want to die.
Second, let's say we don't croak and return undef. We need to set a consistent
variable with the error message. As you saw I've failed to convince p5p to
allow us to peruse $!. And $@ is not appropriate, we have no other choce but
to use some other variable. So what should it be? $Apache::err and
$Apache::errstr, ala $DBI::errstr. Or should there be a separate error
variables for APR and Apache, since APR eventually could be used outside mod_perl?
Also going back to my patch, what do you think about opt_set returning an old
value. I've coded explicitly to get that old value before setting the new one.
That sounds Perlish, but I'm not sure whether it's worth the overhead.
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [mp2 patch] $socket_opt_(get|set) API and test
Posted by Geoffrey Young <ge...@modperlcookbook.org>.
Stas Bekman wrote:
> todays Joe patch triggered my interest to further test the socket
> set/get options, just to discover that opt_get doesn't work at all. So I
> made it working and made both APIs perlish, croacking in case of the
> failure instead of returning status. Sounds good?
I like the idea of returning the value instead of the status - that's
something that we should probably enforce more consistently than we do.
I'm not entirely convinced that croak is a good thing. consider that perl's
native getsockopt() and setsockopt() both return undef on error rather than die.
and I know we've discussed this before, but I really don't like the idea of
croaking at request time - for configuration things it's ok, but the vast
majority of file IO and other operations in perl proper don't die: open,
read, most of the IO:: stuff, etc. in other words, people expect to check
return values for these things instead of needing eval{} so we're throwing
them a curve every time we croak instead.
--Geoff
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org