Re: cvs commit: httpd-2.0/server request.c

[Cliff Woolley <cl...@yahoo.com>]

On 6 Jun 2001 trawick@apache.org wrote:

>   -    if (r->finfo.filetype) {
>   +    if (r->finfo.filetype != APR_NOFILE) {
>    	/* assume path_info already set */

Wow, so we have APR_NOFILE _and_ APR_ENOFILE?  That's awfully confusing,
don't you think?

--Cliff


--------------------------------------------------------------
   Cliff Woolley
   cliffwoolley@yahoo.com
   Charlottesville, VA

Re: cvs commit: httpd-2.0/server request.c

["William A. Rowe, Jr." <ad...@rowe-clan.net>]

From: "Cliff Woolley" <cl...@yahoo.com>
Cc: <de...@apr.apache.org>


> On 6 Jun 2001 trawick@apache.org wrote:
> 
> >   -    if (r->finfo.filetype) {
> >   +    if (r->finfo.filetype != APR_NOFILE) {
> >    /* assume path_info already set */
> 
> Wow, so we have APR_NOFILE _and_ APR_ENOFILE?  That's awfully confusing,
> don't you think?

All the APR_filetype macros should be renamed APR_FTYPE_filetype, to distinguish,
but that implies someone with time has a desire to do so.

And the APR_ENOboo garbage should go away, as Jeff pointed out.

But answering Jeff's side questions, I agree that APR_NOFILE (or APR_FTYPE_NOFILE)
should have a value of zero.

Note that we test filetype rather than perms, which we used to do.  perms don't
exist on every platform in a unixish style, and even where we can test them, it's
very expensive on Win32.  Filetype remains a no-brainer, and a null value (from a
x_calloc() call) should indicate we haven't stat'ted or gotfileinfo, and/or we
tried and found nothing.  Why it ever had a value != 0 is beyond me.

Bill

Re: cvs commit: httpd-2.0/server request.c

[Cliff Woolley <cl...@yahoo.com>]

On 6 Jun 2001, Jeff Trawick wrote:

> > Wow, so we have APR_NOFILE _and_ APR_ENOFILE?  That's awfully confusing,
> > don't you think?
>
> err umm no real opinion as to confusability... one is an error code
> (APR_E*) and one isn't

Yeah, but if you're unfamiliar with the two and haven't actually gone out
and looked at the header files, you might not even notice the 'E'

> clearly APR_ENOFILE is useless and should be removed, so any possible
> confusion should go away

Clearly.  =-)  After that, I don't really have a problem with leaving
APR_NOFILE as it is.

--Cliff

--------------------------------------------------------------
   Cliff Woolley
   cliffwoolley@yahoo.com
   Charlottesville, VA

Re: cvs commit: httpd-2.0/server request.c

[Jeff Trawick <tr...@bellsouth.net>]

Cliff Woolley <cl...@yahoo.com> writes:

> On 6 Jun 2001 trawick@apache.org wrote:
> 
> >   -    if (r->finfo.filetype) {
> >   +    if (r->finfo.filetype != APR_NOFILE) {
> >    	/* assume path_info already set */
> 
> Wow, so we have APR_NOFILE _and_ APR_ENOFILE?  That's awfully confusing,
> don't you think?

err umm no real opinion as to confusability... one is an error code
(APR_E*) and one isn't

clearly APR_ENOFILE is useless and should be removed, so any possible
confusion should go away

whether or not the apache use of r->finfo.filetype is clean is another
matter altogether :)

is it part of the formal API that finfo.filetype being zero means we
didn't find the file?

if so, then why define APR_NOFILE?  just to make the enum include all
valid values?

if not, then does apr_file_stat (or whatever it is) always initialize
finfo.filetype to APR_NOFILE since a caller's memset(,0,)/*calloc() of
the storage didn't initialize it to the right value?

-- 
Jeff Trawick | trawickj@bellsouth.net | PGP public key at web site:
       http://www.geocities.com/SiliconValley/Park/9289/
             Born in Roswell... married an alien...