You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by "William A. Rowe, Jr." <wr...@rowe-clan.net> on 2000/11/29 18:14:52 UTC
SDBM 2.0 'namespace protection'
Question...
I'm again jumping betwixt and between 1.3 and 2.0 - and have
a really simple issue with SDBM... why are we namespace
protecting and modifing this code, given that we now have
apu_dbm as a wrapper?
I'd like to see SDBM go back to a (relatively) pure and
original implementation, and have our wrapper add any namespace
protection and other features (pools, etc.)
Reasonable?
Bill
Re: SDBM 2.0 'namespace protection'
Posted by rb...@covalent.net.
On Thu, 30 Nov 2000, Joe Orton wrote:
> On Wed, Nov 29, 2000 at 03:16:57PM -0800, Ryan Bloom wrote:
> > Pools just aren't the correct solution. Why do you want pools? You say
> > it's to make sure we clean everything up, but that can be done in the
> > sdbm_close function, which can be called from a cleanup that is registered
> > in a wrapper function. Taking a look at where we are using pools in sdbm,
> > it is just for sdbm_prep, which is only called by sdbm_open. There is
> > nothing to cleanup (no cleanups are registered), we only ever allocate a
> > single structure out of this pool.
>
> What about the two calls to apr_open?
But that's just it, those two calls to apr_open could more easily be done
with either just open, or by passing in a NULL pool.
Ryan
_______________________________________________________________________________
Ryan Bloom rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------
Re: SDBM 2.0 'namespace protection'
Posted by Joe Orton <jo...@light.plus.com>.
On Wed, Nov 29, 2000 at 03:16:57PM -0800, Ryan Bloom wrote:
> Pools just aren't the correct solution. Why do you want pools? You say
> it's to make sure we clean everything up, but that can be done in the
> sdbm_close function, which can be called from a cleanup that is registered
> in a wrapper function. Taking a look at where we are using pools in sdbm,
> it is just for sdbm_prep, which is only called by sdbm_open. There is
> nothing to cleanup (no cleanups are registered), we only ever allocate a
> single structure out of this pool.
What about the two calls to apr_open?
joe
Re: SDBM 2.0 'namespace protection'
Posted by Greg Stein <gs...@lyra.org>.
On Thu, Nov 30, 2000 at 10:37:16AM -0800, rbb@covalent.net wrote:
>...
You are so missing the point on all this, and you speak to weird minutia of
my emails, that it is impossible to go anywhere with this discussion about
src/lib/. I talk about something, giving two examples, and you pick one and
blow off the more important/relevant other one.
I'm done with this topic.
>...
> So, can I remove the pools from SDBM then?
No. I'll do the right thing.
-g
--
Greg Stein, http://www.lyra.org/
Re: SDBM 2.0 'namespace protection'
Posted by rb...@covalent.net.
> > This could be done with a simple script, and we don't update those
> > packages all that often. I don't like the argument that this is too
> > difficult if it is the correct solution, and if it is the correct solution
> > for SDBM, then it is the correct solution for PCRE and Expat.
>
> No. We would be changed the APIs of those packages. People would no longer
> be coding against PCRE or Expat, but some Apache API.
>
> SDBM *is* an Apache API. There is no "canonical" package for it. We have
> defined it as using the "sdbm_" prefix.
No, PCRE and Expat are external programs, but we don't distribute those,
we distribute Apache, which happens to use those. If we don't want to
modify the APIs, then we can't compile these packages into Apache, we need
to use shared libraries. We are NOT distributing the "canonical" packages
for Expat or PCRE, we are using them, so changing the APIs, or only
exporting API's that are wrappers around the native wrappers is okay for
us to do.
> > > It is located in lib/sdbm/ because it is a nicely modularized little
> > > library. The same goes for lib/aputil/ (assuming it grows like we've
> > > discussed so many times in the past).
> > >
> > > lib/ does not mean "external package"; it means "a chunk of modularized code
> > > that we can pick up into our build/link". That is a "lib" to me. If we want
> > > to formalize that we're pulling in external packages, then we should have a
> > > subdir named "xpackages/" or something. That isn't on Roy's list, but if
> > > people want that subdir, then we can easily have it added.
> >
> > A chunk of modularized code also means that it can be removed from Apache
> > and still work and make sense. Otherwise, I don't consider it modular.
>
> So standard/mod_foo is not modular in your mind? modules/mpm/prefork/?
> src/ap/? Each of those files/dirs is an independent code entity. Subdirs in
> lib/ are just the same.
No they aren't the same. standard/mod_foo is not a module, it is an
"Apache module". The very type of module ties it to Apache and says that
it won't work outside of Apache. src/lib is not the same thing, this
should be and has been code that can be used completely independantly of
Apache.
> Ryan: lib/ subdirs do not need to be removable. They are simply isolated
> chunks of code that do not necessarily tie into Apache, or have a very light
> coupling (e.g. to APR).
Tying into APR is not the same as tying into Apache, so that isn't an
issue.
> Where else would you put SDBM, expat-lite, and aputil if not lib/? Consider
> that none of those need to be used outside of Apache. Where do they go?
SDBM, expat-lite, and APR are all useful outside of Apache proper. The
proof of this, is that they all have their own build systems, and they are
all used in other programs. Aputil is not useful outside of Apache. It
relies on Apache to build, and as near as I can tell, this code is very
Apache dependant. If this was going to be a wrapper around every
different database implementation, then I could see using it, but for that
I would steal code from PHP, because they have already solved this problem
cleanly. I do not understand the purpose of aputil, especially not as it
currently stands.
> > > Why are pools wrong? You said the same about apu_dbm's internal type. What
> > > is wrong with using pools?! I really don't understand... :-(
> >
> > Pools just aren't the correct solution. Why do you want pools? You say
> > it's to make sure we clean everything up, but that can be done in the
> > sdbm_close function, which can be called from a cleanup that is registered
> > in a wrapper function. Taking a look at where we are using pools in sdbm,
> > it is just for sdbm_prep, which is only called by sdbm_open. There is
> > nothing to cleanup (no cleanups are registered), we only ever allocate a
> > single structure out of this pool. Why aren't we using malloc for
> > this? This memory is essentially never freed as the code stands right
> > now. If we weren't using pools, then the sdbm close function could free
> > the memory.
>
> Great. Now you make sense. I wish you would explain yourself, rather than
> make sweeping announcements like "we shouldn't use pools here!". Doing that
> is just plain antagonistic. Especially when the code has been like that for
> five months.
It really doesn't matter how long the code has been like that, and I have
tried to explain this before. I may not have done as good a job as I did
yesterday, but explaining what I think has never been my strong point.
So, can I remove the pools from SDBM then?
Ryan
_______________________________________________________________________________
Ryan Bloom rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------
Re: SDBM 2.0 'namespace protection'
Posted by Greg Stein <gs...@lyra.org>.
On Wed, Nov 29, 2000 at 03:16:57PM -0800, rbb@covalent.net wrote:
> > > Ummmm..... Expat and PCRE haven't been namespace protected. I personally
> > > believe they should, but they aren't now.
> >
> > Those are external packages. It would be a bitch to keep them maintained if
> > we heavily patched them. SDBM is an Apache-internal package, so we're free
> > to make it as Apache-specific as we'd like.
>
> This could be done with a simple script, and we don't update those
> packages all that often. I don't like the argument that this is too
> difficult if it is the correct solution, and if it is the correct solution
> for SDBM, then it is the correct solution for PCRE and Expat.
No. We would be changed the APIs of those packages. People would no longer
be coding against PCRE or Expat, but some Apache API.
SDBM *is* an Apache API. There is no "canonical" package for it. We have
defined it as using the "sdbm_" prefix.
> > It is located in lib/sdbm/ because it is a nicely modularized little
> > library. The same goes for lib/aputil/ (assuming it grows like we've
> > discussed so many times in the past).
> >
> > lib/ does not mean "external package"; it means "a chunk of modularized code
> > that we can pick up into our build/link". That is a "lib" to me. If we want
> > to formalize that we're pulling in external packages, then we should have a
> > subdir named "xpackages/" or something. That isn't on Roy's list, but if
> > people want that subdir, then we can easily have it added.
>
> A chunk of modularized code also means that it can be removed from Apache
> and still work and make sense. Otherwise, I don't consider it modular.
So standard/mod_foo is not modular in your mind? modules/mpm/prefork/?
src/ap/? Each of those files/dirs is an independent code entity. Subdirs in
lib/ are just the same.
Ryan: lib/ subdirs do not need to be removable. They are simply isolated
chunks of code that do not necessarily tie into Apache, or have a very light
coupling (e.g. to APR).
Where else would you put SDBM, expat-lite, and aputil if not lib/? Consider
that none of those need to be used outside of Apache. Where do they go?
> > > The reason for losing those mods, is that pools are the wrong solution, as
> > > is, I believe APR. APR would be the correct solution if it wasn't tied to
> > > pools, but it is.
> >
> > Why are pools wrong? You said the same about apu_dbm's internal type. What
> > is wrong with using pools?! I really don't understand... :-(
>
> Pools just aren't the correct solution. Why do you want pools? You say
> it's to make sure we clean everything up, but that can be done in the
> sdbm_close function, which can be called from a cleanup that is registered
> in a wrapper function. Taking a look at where we are using pools in sdbm,
> it is just for sdbm_prep, which is only called by sdbm_open. There is
> nothing to cleanup (no cleanups are registered), we only ever allocate a
> single structure out of this pool. Why aren't we using malloc for
> this? This memory is essentially never freed as the code stands right
> now. If we weren't using pools, then the sdbm close function could free
> the memory.
Great. Now you make sense. I wish you would explain yourself, rather than
make sweeping announcements like "we shouldn't use pools here!". Doing that
is just plain antagonistic. Especially when the code has been like that for
five months.
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
Re: SDBM 2.0 'namespace protection'
Posted by rb...@covalent.net.
> > Ummmm..... Expat and PCRE haven't been namespace protected. I personally
> > believe they should, but they aren't now.
>
> Those are external packages. It would be a bitch to keep them maintained if
> we heavily patched them. SDBM is an Apache-internal package, so we're free
> to make it as Apache-specific as we'd like.
This could be done with a simple script, and we don't update those
packages all that often. I don't like the argument that this is too
difficult if it is the correct solution, and if it is the correct solution
for SDBM, then it is the correct solution for PCRE and Expat.
> It is located in lib/sdbm/ because it is a nicely modularized little
> library. The same goes for lib/aputil/ (assuming it grows like we've
> discussed so many times in the past).
>
> lib/ does not mean "external package"; it means "a chunk of modularized code
> that we can pick up into our build/link". That is a "lib" to me. If we want
> to formalize that we're pulling in external packages, then we should have a
> subdir named "xpackages/" or something. That isn't on Roy's list, but if
> people want that subdir, then we can easily have it added.
A chunk of modularized code also means that it can be removed from Apache
and still work and make sense. Otherwise, I don't consider it modular.
> > The reason for losing those mods, is that pools are the wrong solution, as
> > is, I believe APR. APR would be the correct solution if it wasn't tied to
> > pools, but it is.
>
> Why are pools wrong? You said the same about apu_dbm's internal type. What
> is wrong with using pools?! I really don't understand... :-(
Pools just aren't the correct solution. Why do you want pools? You say
it's to make sure we clean everything up, but that can be done in the
sdbm_close function, which can be called from a cleanup that is registered
in a wrapper function. Taking a look at where we are using pools in sdbm,
it is just for sdbm_prep, which is only called by sdbm_open. There is
nothing to cleanup (no cleanups are registered), we only ever allocate a
single structure out of this pool. Why aren't we using malloc for
this? This memory is essentially never freed as the code stands right
now. If we weren't using pools, then the sdbm close function could free
the memory.
Pools are a great memory allocation scheme, but they aren't right for
every situation. In this case, pools don't make any sense, because we
aren't using any of the features that pools give us.
Ryan
_______________________________________________________________________________
Ryan Bloom rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------
Re: SDBM 2.0 'namespace protection'
Posted by Greg Stein <gs...@lyra.org>.
On Wed, Nov 29, 2000 at 11:06:24AM -0800, rbb@covalent.net wrote:
>
> > We namespace protect everything that might be visible, whether it is
> > intended for use by other modules or not. Since the stuff goes into the same
> > process space as other, arbitrary code, we need to protect it (and protect
> > others from it).
>
> Ummmm..... Expat and PCRE haven't been namespace protected. I personally
> believe they should, but they aren't now.
Those are external packages. It would be a bitch to keep them maintained if
we heavily patched them. SDBM is an Apache-internal package, so we're free
to make it as Apache-specific as we'd like.
It is located in lib/sdbm/ because it is a nicely modularized little
library. The same goes for lib/aputil/ (assuming it grows like we've
discussed so many times in the past).
lib/ does not mean "external package"; it means "a chunk of modularized code
that we can pick up into our build/link". That is a "lib" to me. If we want
to formalize that we're pulling in external packages, then we should have a
subdir named "xpackages/" or something. That isn't on Roy's list, but if
people want that subdir, then we can easily have it added.
> > > I'd like to see SDBM go back to a (relatively) pure and
> > > original implementation, and have our wrapper add any namespace
> > > protection and other features (pools, etc.)
[ lost some context: reinserting my comment in here ]
> > There is no such thing as a "pure and original implementation." The code was
> > developed in the early 1990s, made bug free, and effectively abandoned to
> > the Public Domain.
[ done ]
> > I scrounged a copy from somewhere, integrated fixes from Perl, swapped fixes
> > with Ralf (he uses it in mod_ssl), brought the code up to ANSI standards
> > (you wouldn't believe some of the crap that was in there), and then APR-ized
> > it when it went into Apache 2.0 (to get portable file mgmt, portable file
> > locking, and the protection of pools for allocation).
> >
> > I don't see any purpose served by losing any of those mods.
>
> The reason for losing those mods, is that pools are the wrong solution, as
> is, I believe APR. APR would be the correct solution if it wasn't tied to
> pools, but it is.
Why are pools wrong? You said the same about apu_dbm's internal type. What
is wrong with using pools?! I really don't understand... :-(
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
Re: SDBM 2.0 'namespace protection'
Posted by Greg Stein <gs...@lyra.org>.
On Fri, Dec 01, 2000 at 12:10:56AM +0000, Tony Finch wrote:
> rbb@covalent.net wrote:
> >> I don't see any purpose served by losing any of those mods.
> >
> >The reason for losing those mods, is that pools are the wrong solution, as
> >is, I believe APR. APR would be the correct solution if it wasn't tied to
> >pools, but it is.
>
> On the whole the sdbm changes (relative to the sdbm in perl) are
> minor, afaics. I can't see that the use of pools is a problem (mostly
> because sdbm doesn't do much allocation), except in one case: in
> sdbm_open() a malloc() followed almost immediately by a free() was
> changed into allocation from a long-lived pool (i.e. a memory leak).
"long-lived pool" is making a supposition about that pool. In fact, that
pool could be very short lived :-)
[ and concretely: Joe Orton has patched mod_dav to do just that; that patch
will be incorporated into Apache 2.0 in some fashion, but there are a few
things to sort out first. ]
In any case... Tony: update your working copy (I changed that code yesterday
at Ryan's suggestion); the malloc was put back and the memory is freed at
close time or pool cleanup time. The pools are still used for files. (we
also use APR for advisory locks on those files)
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
Re: SDBM 2.0 'namespace protection'
Posted by Tony Finch <do...@dotat.at>.
rbb@covalent.net wrote:
>
>> I don't see any purpose served by losing any of those mods.
>
>The reason for losing those mods, is that pools are the wrong solution, as
>is, I believe APR. APR would be the correct solution if it wasn't tied to
>pools, but it is.
On the whole the sdbm changes (relative to the sdbm in perl) are
minor, afaics. I can't see that the use of pools is a problem (mostly
because sdbm doesn't do much allocation), except in one case: in
sdbm_open() a malloc() followed almost immediately by a free() was
changed into allocation from a long-lived pool (i.e. a memory leak).
Tony.
--
f.a.n.finch dot@dotat.at fanf@covalent.net Chad for President!
Re: SDBM 2.0 'namespace protection'
Posted by rb...@covalent.net.
> We namespace protect everything that might be visible, whether it is
> intended for use by other modules or not. Since the stuff goes into the same
> process space as other, arbitrary code, we need to protect it (and protect
> others from it).
Ummmm..... Expat and PCRE haven't been namespace protected. I personally
believe they should, but they aren't now.
> > I'd like to see SDBM go back to a (relatively) pure and
> > original implementation, and have our wrapper add any namespace
> > protection and other features (pools, etc.)
>
> I scrounged a copy from somewhere, integrated fixes from Perl, swapped fixes
> with Ralf (he uses it in mod_ssl), brought the code up to ANSI standards
> (you wouldn't believe some of the crap that was in there), and then APR-ized
> it when it went into Apache 2.0 (to get portable file mgmt, portable file
> locking, and the protection of pools for allocation).
>
> I don't see any purpose served by losing any of those mods.
The reason for losing those mods, is that pools are the wrong solution, as
is, I believe APR. APR would be the correct solution if it wasn't tied to
pools, but it is.
Ryan
_______________________________________________________________________________
Ryan Bloom rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------
Re: SDBM 2.0 'namespace protection'
Posted by Greg Stein <gs...@lyra.org>.
On Wed, Nov 29, 2000 at 09:14:52AM -0800, William A. Rowe, Jr. wrote:
> Question...
>
> I'm again jumping betwixt and between 1.3 and 2.0 - and have
> a really simple issue with SDBM... why are we namespace
> protecting and modifing this code, given that we now have
> apu_dbm as a wrapper?
Because it is linked into the process, and the symbols are visible from
other things that get pulled into Apache.
We namespace protect everything that might be visible, whether it is
intended for use by other modules or not. Since the stuff goes into the same
process space as other, arbitrary code, we need to protect it (and protect
others from it).
> I'd like to see SDBM go back to a (relatively) pure and
> original implementation, and have our wrapper add any namespace
> protection and other features (pools, etc.)
There is no such thing as a "pure and original implementation." The code was
developed in the early 1990s, made bug free, and effectively abandoned to
the Public Domain.
I scrounged a copy from somewhere, integrated fixes from Perl, swapped fixes
with Ralf (he uses it in mod_ssl), brought the code up to ANSI standards
(you wouldn't believe some of the crap that was in there), and then APR-ized
it when it went into Apache 2.0 (to get portable file mgmt, portable file
locking, and the protection of pools for allocation).
I don't see any purpose served by losing any of those mods.
Cheers,
-g
--
Greg Stein, http://www.lyra.org/