Ok, this is a sparkling demonstration of the weakness behind
any validation scheme. Let's refactor it a few times.
> wrowe 01/01/20 13:40:23
>
> diff -u -r1.3 -r1.4
> --- sdbm.c 2000/12/12 08:54:30 1.3
> +++ sdbm.c 2001/01/20 21:40:23 1.4
> @@ -177,7 +177,8 @@
> /*
> * need the dirfile size to establish max bit number.
> */
> - if ((status = apr_getfileinfo(&finfo, db->dirf)) != APR_SUCCESS)
> + if (((status = apr_getfileinfo(&finfo, APR_FINFO_SIZE, db->dirf))
> + != APR_SUCCESS) || !(finfo.valid & APR_FINFO_SIZE))
> goto error;
Simple, and elegant. Of course this would be simpler;
> + if (((status = apr_getfileinfo(&finfo, APR_FINFO_SIZE, db->dirf))
> + != APR_SUCCESS))
Now the underlying issue gets more complex when we ask
if (((status = apr_getfileinfo(&finfo, APR_FINFO_NORM, db->dirf))
!= APR_SUCCESS))
We expect most everything, but some platforms will be missing something
(AIX has no group, correct?) Rather than keep -lying- to the application,
apr ought to be honest and leave those bits unset. But we do -not- want
to fail in this case. What if we add a bit of cruft?
if (((status = apr_getfileinfo(&finfo, APR_FINFO_NORM, db->dirf))
!= APR_SUCCESS) && status != APR_INCOMPLETE)
This is ugly as well, now we have two tests where 99% of the time we
just don't care. If we would 'like' to do something about the user
of the file later, we still need to do this...
if (finfo.valid & APR_FINFO_USER)
something(finfo.user);
So what's the pain v.s. gain of reporting APR_INCOMPLETE?
If we want to be notified, I'd suggest the high bit of the wanted arg
is passed as APR_FINFO_FAIL. When we -must- have a user, we can do
this instead;
if (((status = apr_getfileinfo(&finfo, APR_FINFO_USER | APR_FINFO_FAIL,
db->dirf)) != APR_SUCCESS))
Pretty gosh darned straightforward. But so is the current;
if (((status = apr_getfileinfo(&finfo, APR_FINFO_USER, db->dirf))
!= APR_SUCCESS) || !(finfo.valid & APR_FINFO_USER))
Thoughts, folks?
