You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@perl.apache.org by do...@pobox.com on 2000/07/25 01:15:50 UTC
Re: PL_laststatval
On Mon, 24 Jul 2000, Eric Cholet wrote:
> Hi,
>
> I want to fix this bug in r->filename. Currently it does this:
>
> CODE:
> get_set_PVp(r->filename,r->pool);
> #ifndef WIN32
> if(items > 1)
> stat(r->filename, &r->finfo);
> #endif
>
> The return code for stat() is not checked, therefore the stat
> cache is left at its previous value which doesn't seem right.
> What I want to do is this:
> CODE:
> get_set_PVp(r->filename,r->pool);
> #ifndef WIN32
> if(items > 1)
> - stat(r->filename, &r->finfo);
> + laststatval = stat(r->filename, &r->finfo);
> #endif
missing one piece from roger's patch, something like so:
if ((laststatval = stat(...)) < 0) {
r->finfo.st_mode = 0;
}
> What bothers me is this code in r->finfo:
>
> CODE:
> statcache = r->finfo;
> if (r->finfo.st_mode) {
> laststatval = 0;
> }
> else {
> laststatval = -1;
> }
>
> Why is it checking st_mode instead of laststatval ? That seems wrong,
> if the last stat() returned -1 then r->finfo.st_mode shouldn't be
> trusted, no?
$r->finfo shouldn't check laststatval, Apache did the stat() for
&r->finfo, which doesn not know about PL_laststatval. r->finfo.st_mode
should always be 0 if the stat() of r->filename failed, see
http_request.c:
/* must set this to zero, some stat()s may have corrupted it
* even if they returned an error.
*/
r->finfo.st_mode = 0;
and http_core.c tests that value:
if (r->finfo.st_mode == 0 || (r->path_info && *r->path_info)) {
ap_log_rerror(APLOG_MARK, APLOG_ERR|APLOG_NOERRNO, r,
"File does not exist: %s",r->path_info ?
ap_pstrcat(r->pool, r->filename, r->path_info, NULL)
: r->filename);
return HTTP_NOT_FOUND;
}
i think we're ok to use that test for $r->finfo
Re: PL_laststatval
Posted by do...@pobox.com.
On Tue, 25 Jul 2000, Eric Cholet wrote:
> Doug,
>
> Thanks for the insight, it all makes sense now. Therefore the patch I propose
> to commit is:
looks good, thanks eric!
Re: PL_laststatval
Posted by Eric Cholet <ch...@logilune.com>.
> > I want to fix this bug in r->filename. Currently it does this:
> >
> > CODE:
> > get_set_PVp(r->filename,r->pool);
> > #ifndef WIN32
> > if(items > 1)
> > stat(r->filename, &r->finfo);
> > #endif
> >
> > The return code for stat() is not checked, therefore the stat
> > cache is left at its previous value which doesn't seem right.
> > What I want to do is this:
> > CODE:
> > get_set_PVp(r->filename,r->pool);
> > #ifndef WIN32
> > if(items > 1)
> > - stat(r->filename, &r->finfo);
> > + laststatval = stat(r->filename, &r->finfo);
> > #endif
>
> missing one piece from roger's patch, something like so:
>
> if ((laststatval = stat(...)) < 0) {
> r->finfo.st_mode = 0;
> }
>
> > What bothers me is this code in r->finfo:
> >
> > CODE:
> > statcache = r->finfo;
> > if (r->finfo.st_mode) {
> > laststatval = 0;
> > }
> > else {
> > laststatval = -1;
> > }
> >
> > Why is it checking st_mode instead of laststatval ? That seems wrong,
> > if the last stat() returned -1 then r->finfo.st_mode shouldn't be
> > trusted, no?
>
> $r->finfo shouldn't check laststatval, Apache did the stat() for
> &r->finfo, which doesn not know about PL_laststatval. r->finfo.st_mode
> should always be 0 if the stat() of r->filename failed, see
> http_request.c:
>
> /* must set this to zero, some stat()s may have corrupted it
> * even if they returned an error.
> */
> r->finfo.st_mode = 0;
>
> and http_core.c tests that value:
>
> if (r->finfo.st_mode == 0 || (r->path_info && *r->path_info)) {
> ap_log_rerror(APLOG_MARK, APLOG_ERR|APLOG_NOERRNO, r,
> "File does not exist: %s",r->path_info ?
> ap_pstrcat(r->pool, r->filename, r->path_info, NULL)
> : r->filename);
> return HTTP_NOT_FOUND;
> }
>
> i think we're ok to use that test for $r->finfo
Doug,
Thanks for the insight, it all makes sense now. Therefore the patch I propose
to commit is:
Index: Apache.xs
===================================================================
RCS file: /home/cvs/modperl/src/modules/perl/Apache.xs,v
retrieving revision 1.98
diff -u -r1.98 Apache.xs
--- Apache.xs 2000/06/05 18:18:49 1.98
+++ Apache.xs 2000/07/25 09:43:54
@@ -1870,7 +1870,9 @@
get_set_PVp(r->filename,r->pool);
#ifndef WIN32
if(items > 1)
- stat(r->filename, &r->finfo);
+ if ((laststatval = stat(r->filename, &r->finfo)) < 0) {
+ r->finfo.st_mode = 0;
+ }
#endif
OUTPUT:
I'm just double-checking 'cuz I don't want to break anything in
this sensitive area.
Thanks,
--
Eric