You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@apr.apache.org by wr...@apache.org on 2001/01/20 22:40:23 UTC
cvs commit: apr-util/dbm/sdbm sdbm.c
wrowe 01/01/20 13:40:23
Modified: dbm/sdbm sdbm.c
Log:
apr-util changes to support APR_FINFO_wanted declaration changes for
apr_stat/lstat/getfileinfo. These are not -optimal- changes, simply
the straight line to get the server building again.
Revision Changes Path
1.4 +2 -1 apr-util/dbm/sdbm/sdbm.c
Index: sdbm.c
===================================================================
RCS file: /home/cvs/apr-util/dbm/sdbm/sdbm.c,v
retrieving revision 1.3
retrieving revision 1.4
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;
/*
Re: cvs commit: apr-util/dbm/sdbm sdbm.c
Posted by Greg Stein <gs...@lyra.org>.
On Sat, Jan 20, 2001 at 04:49:57PM -0600, William A. Rowe, Jr. wrote:
> > From: Greg Stein [mailto:gstein@lyra.org]
> >
> > [ but most code can't be flexible ]
>
> Then it isn't written to be portable. apr is meaningless to such an app.
That is a silly position to take.
If I ask for the size of a file because I need it, and APR doesn't give it
to me, then that is an error. If I tell APR that I can be flexible, and can
somehow manage without it (because I've written extra code), then great.
APR needs to give me what I ask for, if it can. Whether that makes me
portable or not is NOT up to APR to decide.
APR defines a policy where it can refuse to return data, and it will advise
me of such. But I should not have to code a bazillion workarounds in the
common case. If I ask for something, and it doesn't come back: that is an
error. The code looks like:
if (apr_getfileinfo(...) != APR_SUCCESS)
/* ### error */
If I want to make my app flexible in what it can use, and what it can safely
NOT receive from APR, then I have that choice. But recognize that it
requires *extra* programming on the app's part to be flexible.
In the current scheme, I need extra programming for all cases:
1) to deal with the case of APR_SUCCESS and some needed data isn't present
2) to properly deal with missing data (when that is okay)
I'd rather see extra programming only for the "I'm flexible" case. That also
returns the semantics of apr_stat() to the original model.
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
Re: cvs commit: apr-util/dbm/sdbm sdbm.c
Posted by Greg Stein <gs...@lyra.org>.
I'd like to see apr_stat's semantics returned to before. If the apr_stat()
succeeded, then the requested data was made available.
Adding a flag to tell APR "hey, I'm cool with partial data" is the right way
for an app let APR know that it can compensate for missing data.
Cheers,
-g
On Sat, Jan 20, 2001 at 02:40:44PM -0800, Greg Stein wrote:
> On Sat, Jan 20, 2001 at 04:23:24PM -0600, William A. Rowe, Jr. wrote:
> >...
> > 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?
>
> Reverse the logic.
>
> In most cases, the caller wants exactly what was asked for. *If* the caller
> can deal with some items left out, then they can declare that.
>
> Thus, you have:
>
> if ((status = apr_getfileinfo(&finfo, APR_FINFO_USER, db->dirf))
> == APR_SUCCESS)
> /* we know we have the info */
>
> or:
>
> if ((status = apr_getfileinfo(&finfo, APR_FINFO_USER | APR_FINFO_FLEXIBLE,
> db->dirf)) == APR_SUCCESS)
> /* we have some/all of the info. look at finfo.valid */
>
> When _FLEXIBLE is passed, apr_getfileinfo() will almost never fail. It would
> need to be pretty drastic :-)
>
> In the above scheme, all of the conversions that you did will operate just
> like they used to. For code that can be flexible with the values, they have
> that chance.
>
> [ but most code can't be flexible ]
>
> Cheers,
> -g
>
> --
> Greg Stein, http://www.lyra.org/
--
Greg Stein, http://www.lyra.org/
Re: cvs commit: apr-util/dbm/sdbm sdbm.c
Posted by Greg Stein <gs...@lyra.org>.
On Sat, Jan 20, 2001 at 04:23:24PM -0600, William A. Rowe, Jr. wrote:
>...
> 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?
Reverse the logic.
In most cases, the caller wants exactly what was asked for. *If* the caller
can deal with some items left out, then they can declare that.
Thus, you have:
if ((status = apr_getfileinfo(&finfo, APR_FINFO_USER, db->dirf))
== APR_SUCCESS)
/* we know we have the info */
or:
if ((status = apr_getfileinfo(&finfo, APR_FINFO_USER | APR_FINFO_FLEXIBLE,
db->dirf)) == APR_SUCCESS)
/* we have some/all of the info. look at finfo.valid */
When _FLEXIBLE is passed, apr_getfileinfo() will almost never fail. It would
need to be pretty drastic :-)
In the above scheme, all of the conversions that you did will operate just
like they used to. For code that can be flexible with the values, they have
that chance.
[ but most code can't be flexible ]
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
RE: cvs commit: apr-util/dbm/sdbm sdbm.c
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
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?