perl-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Stas Bekman <s...@stason.org>
Subject Re: additional APR::Status::is_* functions?
Date Sun, 08 May 2005 04:35:27 GMT
Randy Kobes wrote:
> On Sat, 7 May 2005, Stas Bekman wrote:
> 
> 
>>Randy Kobes wrote:
>>
>>>In the t/protocol/ tests, there's some comparisions of $@ to
>>>APR::Const::ECONNABORTED and APR::Const::EOF made. As such,
>>>it may be an idea to add and use
>>>APR::Status::is_ECONNABORTED and APR::Status::is_EOF
>>>instead. At the present time APR_EOF doesn't have any
>>>variants:
>>>  #define APR_STATUS_IS_EOF(s) ((s) == APR_EOF)
>>>But for consistency with the other uses of APR::Status,
>>>would it be an idea to add this anyway?
>>
>>>This would
>>>also handle possible changes to APR_STATUS_IS_EOF(s)
>>>in the future.
>>
>>+1 for ECONNABORTED, as it's already a composite
>>
>>not sure about EOF (not a composite at the moment), may be
>>ask at apr-dev?
>>
>>
>>>In the same vein, there are variants in the
>>>APR_STATUS_IS_SUCCESS(s) macro. This hasn't been a
>>>problem yet, but should we add an APR::Status::is_SUCCESS
>>>to be used in place of comparison to APR::SUCCESS?
>>
>>Hmm, the API starts to get confusing with having two ways.
>>Again I'd ask @apr-dev if this is essential, but it looks
>>like we need to do it, as it's again a composite.
> 
> 
> Actually, in browsing the apr-dev mail archives, there was a
> discussion of the APR_STATUS_IS_SUCCESS() macro,
> particularly about dumping it. It's now gone in the current
> apr svn trunk. So I guess we should leave things as
> they are, and just compare with APR::SUCCESS?

+1, thanks for doing the research, Randy!

> However, most of the other APR_STATUS_IS_* macros have this
> warning in apr_errno.h:
>   @warning
>    * always use this test, as platform-specific variances
>    * may meet this more than one error code
> as well as comments after the definition of the error
> code like
>   @see APR_STATUS_IS_ECONNABORTED
>    * @warning use APR_STATUS_IS_ECONNABORTED instead of just
>    * testing this value
> even for those for which there's no composite, at present.
> As you say, it is confusing having two ways of accessing
> these, but perhaps we should follow apr's guide in this
> respect, and provide APR::Status::is_* for those macros
> which contain this recommendation for use (for the
> APR::Const::* provided)?

I have no problem with that, just trying to keep the API we expose to the 
required minimum. The more is exposed the more maintenance is required. 
That's why I've suggested to ask at the apr list. I won't be surprised if 
that warning was just copy-n-paste thing. If it was added 4 years ago and 
until today it's still not a composite, chances are that it shouldn't be 
one. but again, it's the safest to ask the list.

In any case, we can always add new macros are 2.0.0 release.

>>Also while fixing docs, please locate the standalone constants in the
>>APR::Const manpage and add a link to the corresponding APR::Status macros
>>(and a reversed link to), see the APR::Const::EAGAIN and
>>APR::Status::is_EAGAIN pod entries for an example.
> 
> 
> I'll do that - thanks.

Thanks, Randy!

it looks that all anchors in xrefs in the docs to *::Const::* are broken.


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


Mime
View raw message