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/09/18 02:30:03 UTC
dropping $finfo->pool?
$finfo->pool (APR::Finfo) seems to be used for internal purposes, I can't
see how users can benefit from having this record entry exposed, besides
confusing them. So I propose to drop it.
--
__________________________________________________________________
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: dropping $finfo->pool?
Posted by Stas Bekman <st...@stason.org>.
$finfo->pool is no more.
--
__________________________________________________________________
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: dropping $finfo->pool?
Posted by "Philippe M. Chiasson" <go...@ectoplasm.org>.
Joe Schaefer wrote:
>Stas Bekman <st...@stason.org> writes:
>
>[...]
>
>
>
>>I don't understand what this pool is for. the C docs for
>>the record entry are totally unclear.
>>
>>
>
>Since apr_file_t is an opaque structure in C, the
>only way to extract any information about it is by
>calling apr_file_info_get() and inspecting the
>corresponding apr_finfo_t struct. Otherwise the
>pool slot serves no useful purpose:
>
> % grep -r "finfo->pool" .
> ./file_io/netware/filestat.c: finfo->pool = thefile->pool;
> ./file_io/netware/filestat.c: finfo->pool = pool;
> ./file_io/os2/dir.c: finfo->pool = thedir->pool;
> ./file_io/unix/dir.c: finfo->pool = thedir->pool;
> ./file_io/unix/filestat.c: finfo->pool = thefile->pool;
> ./file_io/unix/filestat.c: finfo->pool = pool;
> ./file_io/win32/dir.c: finfo->pool = thedir->pool;
> ./file_io/win32/filestat.c: apr_pool_cleanup_register(finfo->pool, pdesc, free_localheap,
> ./file_io/win32/filestat.c: finfo->pool = thefile->pool;
> ./file_io/win32/filestat.c: finfo->pool = pool;
> ./file_io/win32/filestat.c: finfo->pool = pool;
>
>
Feels to me like there is not much motivation for opening up finfo->pool
then
Re: dropping $finfo->pool?
Posted by Geoffrey Young <ge...@modperlcookbook.org>.
> IMO it makes much more sense to have a read-only pool()
> method for APR::File than it does to expose finfo->pool.
ok, I'll believe you :)
> apr_finfo_t doesn't have any semantics outside of being
> a return value of some C function. I'm not sure why there's
> even a perl API for it in the first place.
when the default map_to_storage happens apache has done the filesystem stat
already, which means that over in perl-land we can query $r->finfo instead
of performing a stat on $r->filename and save another trip to disk. it's
also required if you want to set $r->filename yourself and want to properly
interact with default_handler.
longer explanations of each issue can be found here:
http://use.perl.org/~geoff/journal/16002
http://use.perl.org/~geoff/journal/16159
--Geoff
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: dropping $finfo->pool?
Posted by Joe Schaefer <jo...@sunstarsys.com>.
Geoffrey Young <ge...@modperlcookbook.org> writes:
[...]
> > ./file_io/win32/filestat.c:
apr_pool_cleanup_register(finfo->pool, pdesc, free_localheap,
>
> that is certainly an interesting use :)
Nah- The pool in question comes from apr_stat, which
is supplied by the user. It's only used above to
postpone the LocalFree()s on all the GetSecurityInfo
objects. more_finfo could have been written more clearly
with an extra pool argument, instead of getting the original
argument from finfo->pool.
IMO it makes much more sense to have a read-only pool()
method for APR::File than it does to expose finfo->pool.
apr_finfo_t doesn't have any semantics outside of being
a return value of some C function. I'm not sure why there's
even a perl API for it in the first place.
--
Joe Schaefer
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: dropping $finfo->pool?
Posted by Geoffrey Young <ge...@modperlcookbook.org>.
Joe Schaefer wrote:
> Stas Bekman <st...@stason.org> writes:
>
> [...]
>
>
>>I don't understand what this pool is for. the C docs for
>>the record entry are totally unclear.
>
>
> Since apr_file_t is an opaque structure in C, the
> only way to extract any information about it is by
> calling apr_file_info_get() and inspecting the
> corresponding apr_finfo_t struct. Otherwise the
> pool slot serves no useful purpose:
> ./file_io/win32/filestat.c: apr_pool_cleanup_register(finfo->pool, pdesc, free_localheap,
that is certainly an interesting use :)
as philippe mentioned, if finfo->pool is clearly harmful then we should
probably get rid of it. however, if people can add (meaningful) cleanups on
the pool then that sounds like a good reason to keep it around, or at least
a reason to not close it off just for the sake of closing it off.
--Geoff
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: dropping $finfo->pool?
Posted by Joe Schaefer <jo...@sunstarsys.com>.
Stas Bekman <st...@stason.org> writes:
[...]
> I don't understand what this pool is for. the C docs for
> the record entry are totally unclear.
Since apr_file_t is an opaque structure in C, the
only way to extract any information about it is by
calling apr_file_info_get() and inspecting the
corresponding apr_finfo_t struct. Otherwise the
pool slot serves no useful purpose:
% grep -r "finfo->pool" .
./file_io/netware/filestat.c: finfo->pool = thefile->pool;
./file_io/netware/filestat.c: finfo->pool = pool;
./file_io/os2/dir.c: finfo->pool = thedir->pool;
./file_io/unix/dir.c: finfo->pool = thedir->pool;
./file_io/unix/filestat.c: finfo->pool = thefile->pool;
./file_io/unix/filestat.c: finfo->pool = pool;
./file_io/win32/dir.c: finfo->pool = thedir->pool;
./file_io/win32/filestat.c: apr_pool_cleanup_register(finfo->pool, pdesc, free_localheap,
./file_io/win32/filestat.c: finfo->pool = thefile->pool;
./file_io/win32/filestat.c: finfo->pool = pool;
./file_io/win32/filestat.c: finfo->pool = pool;
--
Joe Schaefer
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: dropping $finfo->pool?
Posted by Stas Bekman <st...@stason.org>.
Philippe M. Chiasson wrote:
> Stas Bekman wrote:
>
>> $finfo->pool (APR::Finfo) seems to be used for internal purposes, I
>> can't see how users can benefit from having this record entry exposed,
>> besides confusing them. So I propose to drop it.
>>
> Well, being that's it's just a pool, I can't see a good reason why we'd
> want to _hide_ it away. Can't think of a good
> reason to use $finfo->pool, but I am pretty sure geoff could come up
> with a good example.
>
> I'd say we keep it there unless it's clearly _dangerous_
I don't understand what this pool is for. the C docs for the record entry
are totally unclear. So I propose to pull it off. If you know what it is
for, please document it and then it's a go.
--
__________________________________________________________________
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: dropping $finfo->pool?
Posted by "Philippe M. Chiasson" <go...@ectoplasm.org>.
Stas Bekman wrote:
> $finfo->pool (APR::Finfo) seems to be used for internal purposes, I
> can't see how users can benefit from having this record entry exposed,
> besides confusing them. So I propose to drop it.
>
Well, being that's it's just a pool, I can't see a good reason why we'd
want to _hide_ it away. Can't think of a good
reason to use $finfo->pool, but I am pretty sure geoff could come up
with a good example.
I'd say we keep it there unless it's clearly _dangerous_