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/05/22 02:39:18 UTC
[mp2] APR::Finfo::stat needs rework
Geoff,
APR::Finfo::stat() doesn't do what C API does. The C API populates finfo for
the file passed in the argument. The current perl API doesn't do that. It
populates only $r->finfo, passed as an argument, which may not exist at all.
So it needs to be reworked to create that structure, pass it to apr_stat and
return it to the user, not pass it as an argument. Unless you want to work on
this by yourself, I'll adjust it shortly.
Let me know if I have missed something.
--
__________________________________________________________________
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] APR::Finfo::stat needs rework
Posted by Stas Bekman <st...@stason.org>.
Geoffrey Young wrote:
>>Well, populating $r->finfo is not a job of APR::Finfo which must be
>>independent from httpd.
>
>
> of course, we both know that.
>
> nevertheless, APR::Finfo::stat() pretty much exists for exactly that purpose
> - if a perl user wants to stat a file they already have perl's stat, which
> has been proven much better cross platform anyway :)
True, sans a few bits unique to apr's finfo struct
>>Next do:
>>
>>$r->finfo($finfo);
>
>
> yes, that's the missing piece.
>
> so, +1 to change the API.
Thanks.
--
__________________________________________________________________
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] APR::Finfo::stat needs rework
Posted by Geoffrey Young <ge...@modperlcookbook.org>.
> Well, populating $r->finfo is not a job of APR::Finfo which must be
> independent from httpd.
of course, we both know that.
nevertheless, APR::Finfo::stat() pretty much exists for exactly that purpose
- if a perl user wants to stat a file they already have perl's stat, which
has been proven much better cross platform anyway :)
> Next do:
>
> $r->finfo($finfo);
yes, that's the missing piece.
so, +1 to change the API.
--Geoff
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [mp2] APR::Finfo::stat needs rework
Posted by Stas Bekman <st...@stason.org>.
Stas Bekman wrote:
>> - if you are creating your own apr_finfo_t, then how does one populate
>> r->finfo?
BTW, why would you want to populate $r->finfo? Apache does that already,
doesn't it?
Also I've made all the finfo accessors read-only. Is that OK?
Thinking more, I think we may not do $r->finfo($finfo), but make it do stat()
internally if $r->finfo is not populated. It should be enough to do:
$r->filename('xxx');
$finfo = $r->finfo();
and not needing an explicit stat() call in the perl space.
--
__________________________________________________________________
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] APR::Finfo::stat needs rework
Posted by Stas Bekman <st...@stason.org>.
Geoffrey Young wrote:
>
> Stas Bekman wrote:
>
>>Stas Bekman wrote:
>>
>>
>>>Geoff,
>>>
>>>APR::Finfo::stat() doesn't do what C API does. The C API populates
>>>finfo for the file passed in the argument. The current perl API
>>>doesn't do that. It populates only $r->finfo, passed as an argument,
>>>which may not exist at all.
>
>
> I really don't understand what you mean here. APR::Finfo::stat() is
> autogenerated, so it behaves _exactly_ as the C API does.
>
>
>>+ my $finfo = APR::Finfo::stat($file, APR::FINFO_NORM, $r->pool);
>
>
> the C apr_stat() function takes an apr_finfo_t structure as the first
> argument. where are you getting your apr_finfo_t from? wherever it is that
> you are doing it, as I see it you are creating one of two problems.
>
> - if you're using r->finfo() then you're presupposing a request_rec in an
> APR routine, as well as limiting any other use of apr_stat() that doesn't
> involve r->finfo.
>
> - if you are creating your own apr_finfo_t, then how does one populate
> r->finfo? you absolutely need a way to do that otherwise you can never
> update the data that apache uses to make certain decisions, which is the
> entire point of populating r->finfo via $r->finfo.
>
>
>>- my $status = $r->finfo->stat($file, APR::FINFO_NORM, $r->pool);
>
>
> so, this looks just fine to me. I suppose if you want to stat another file
> you need to figure out a way to generate an apr_finfo_t structure on your
> own, as there currently isn't any method I can see that returns an
> apr_finfo_t other than $r->finfo(). but that doesn't mean we shouldn't add
> one :)
Well, populating $r->finfo is not a job of APR::Finfo which must be
independent from httpd. The adjusted method does:
static MP_INLINE
apr_finfo_t *mpxs_APR__Finfo_stat(pTHX_ const char *fname,
apr_int32_t wanted, apr_pool_t *p)
{
apr_finfo_t *finfo = (apr_finfo_t *)apr_pcalloc(p, sizeof(apr_finfo_t));
MP_RUN_CROAK(apr_stat(finfo, fname, wanted, p),
"APR::IpSubnet::new");
return finfo;
}
Yes, it returns that structure.
Next do:
$r->finfo($finfo);
and you are all set. of course after adjusting this:
static MP_INLINE
apr_finfo_t *mpxs_Apache__RequestRec_finfo(request_rec *r)
{
return &r->finfo;
}
to be settable.
--
__________________________________________________________________
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] APR::Finfo::stat needs rework
Posted by Stas Bekman <st...@stason.org>.
Geoffrey Young wrote:
>
> Geoffrey Young wrote:
>
>>Stas Bekman wrote:
>>
>>
>>>Stas Bekman wrote:
>>>
>>>
>>>
>>>>Geoff,
>>>>
>>>>APR::Finfo::stat() doesn't do what C API does. The C API populates
>>>>finfo for the file passed in the argument. The current perl API
>>>>doesn't do that. It populates only $r->finfo, passed as an argument,
>>>>which may not exist at all.
>>
>>
>>I really don't understand what you mean here. APR::Finfo::stat() is
>>autogenerated, so it behaves _exactly_ as the C API does.
>
>
> oh, I think I see where you are coming from. you wanted to return an object
> instead of a status, just like we had discussed, right?
No, this is totally unrelated.
> the problem is that apr_stat doesn't merely use the apr_finfo_t as a means
> of returning stat information, it populates a existing apr_finfo_t for you
> (like r->finfo). so, I suppose we could have it return an $finfo object,
> but then we still are in need of a way to populate $r->finfo with the stat
> structure. maybe $r->finfo($finfo), but I'm not sure how difficult that
> would be to integrate - I haven't looked at that code in a while and can't
> remember offhand.
Yes, exactly. Why should it be hard to integrate?
--
__________________________________________________________________
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] APR::Finfo::stat needs rework
Posted by Geoffrey Young <ge...@modperlcookbook.org>.
Geoffrey Young wrote:
>
> Stas Bekman wrote:
>
>>Stas Bekman wrote:
>>
>>
>>>Geoff,
>>>
>>>APR::Finfo::stat() doesn't do what C API does. The C API populates
>>>finfo for the file passed in the argument. The current perl API
>>>doesn't do that. It populates only $r->finfo, passed as an argument,
>>>which may not exist at all.
>
>
> I really don't understand what you mean here. APR::Finfo::stat() is
> autogenerated, so it behaves _exactly_ as the C API does.
oh, I think I see where you are coming from. you wanted to return an object
instead of a status, just like we had discussed, right?
the problem is that apr_stat doesn't merely use the apr_finfo_t as a means
of returning stat information, it populates a existing apr_finfo_t for you
(like r->finfo). so, I suppose we could have it return an $finfo object,
but then we still are in need of a way to populate $r->finfo with the stat
structure. maybe $r->finfo($finfo), but I'm not sure how difficult that
would be to integrate - I haven't looked at that code in a while and can't
remember offhand.
--Geoff
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [mp2] APR::Finfo::stat needs rework
Posted by Geoffrey Young <ge...@modperlcookbook.org>.
Stas Bekman wrote:
> Stas Bekman wrote:
>
>> Geoff,
>>
>> APR::Finfo::stat() doesn't do what C API does. The C API populates
>> finfo for the file passed in the argument. The current perl API
>> doesn't do that. It populates only $r->finfo, passed as an argument,
>> which may not exist at all.
I really don't understand what you mean here. APR::Finfo::stat() is
autogenerated, so it behaves _exactly_ as the C API does.
> + my $finfo = APR::Finfo::stat($file, APR::FINFO_NORM, $r->pool);
the C apr_stat() function takes an apr_finfo_t structure as the first
argument. where are you getting your apr_finfo_t from? wherever it is that
you are doing it, as I see it you are creating one of two problems.
- if you're using r->finfo() then you're presupposing a request_rec in an
APR routine, as well as limiting any other use of apr_stat() that doesn't
involve r->finfo.
- if you are creating your own apr_finfo_t, then how does one populate
r->finfo? you absolutely need a way to do that otherwise you can never
update the data that apache uses to make certain decisions, which is the
entire point of populating r->finfo via $r->finfo.
> - my $status = $r->finfo->stat($file, APR::FINFO_NORM, $r->pool);
so, this looks just fine to me. I suppose if you want to stat another file
you need to figure out a way to generate an apr_finfo_t structure on your
own, as there currently isn't any method I can see that returns an
apr_finfo_t other than $r->finfo(). but that doesn't mean we shouldn't add
one :)
--Geoff
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [mp2] APR::Finfo::stat needs rework
Posted by Stas Bekman <st...@stason.org>.
Stas Bekman wrote:
> Geoff,
>
> APR::Finfo::stat() doesn't do what C API does. The C API populates finfo
> for the file passed in the argument. The current perl API doesn't do
> that. It populates only $r->finfo, passed as an argument, which may not
> exist at all.
>
> So it needs to be reworked to create that structure, pass it to apr_stat
> and return it to the user, not pass it as an argument. Unless you want
> to work on this by yourself, I'll adjust it shortly.
>
> Let me know if I have missed something.
I've implemented it, please take a look at the adjusted test using the fixed
stat API:
Index: t/response/TestAPR/finfo.pm
===================================================================
RCS file: /home/cvs/modperl-2.0/t/response/TestAPR/finfo.pm,v
retrieving revision 1.10
diff -u -r1.10 finfo.pm
--- t/response/TestAPR/finfo.pm 3 May 2004 12:54:08 -0000 1.10
+++ t/response/TestAPR/finfo.pm 22 May 2004 01:04:08 -0000
@@ -44,16 +44,13 @@
}
my $file = $r->server_root_relative(catfile qw(htdocs index.html));
+ # populate the finfo struct first
+ my $finfo = APR::Finfo::stat($file, APR::FINFO_NORM, $r->pool);
- # stat tests
- {
- # populate the finfo struct first
- my $status = $r->finfo->stat($file, APR::FINFO_NORM, $r->pool);
-
- ok t_cmp(APR::SUCCESS,
- $status,
- "stat $file");
+ ok $finfo->isa('APR::Finfo');
+ # stat tests (same as perl's stat)
+ {
# now, get information from perl's stat()
our ($device, $inode, $protection, $nlink, $user, $group,
undef, $size, $atime, $mtime, $ctime) = stat $file;
@@ -81,8 +78,8 @@
}
else {
ok t_cmp(${$method},
- $r->finfo->$method(),
- "\$r->finfo->$method()");
+ $finfo->$method(),
+ "\$finfo->$method()");
}
}
}
@@ -90,20 +87,20 @@
# match world bits
ok t_cmp($protection & S_IROTH,
- $r->finfo->protection & APR::WREAD,
- '$r->finfo->protection() & APR::WREAD');
+ $finfo->protection & APR::WREAD,
+ '$finfo->protection() & APR::WREAD');
ok t_cmp($protection & S_IWOTH,
- $r->finfo->protection & APR::WWRITE,
- '$r->finfo->protection() & APR::WWRITE');
+ $finfo->protection & APR::WWRITE,
+ '$finfo->protection() & APR::WWRITE');
if (WIN32) {
skip "different file semantics", 0;
}
else {
ok t_cmp($protection & S_IXOTH,
- $r->finfo->protection & APR::WEXECUTE,
- '$r->finfo->protection() & APR::WEXECUTE');
+ $finfo->protection & APR::WEXECUTE,
+ '$finfo->protection() & APR::WEXECUTE');
}
}
@@ -115,13 +112,13 @@
}
else {
ok t_cmp($file,
- $r->finfo->fname,
- '$r->finfo->fname()');
+ $finfo->fname,
+ '$finfo->fname()');
}
ok t_cmp(APR::REG,
- $r->finfo->filetype,
- '$r->finfo->filetype()');
+ $finfo->filetype,
+ '$finfo->filetype()');
}
Apache::OK;
--
__________________________________________________________________
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