PR41190 - which fix should we use

[Niklas Edmundsson <ni...@acc.umu.se>]

Hi!

Since I no longer have an excuse I thought I should have a go at 
fixing PR41190, mod_rewrite stat:ing the wrong file when using sdbm 
maps.

Read http://issues.apache.org/bugzilla/show_bug.cgi?id=41190 for 
the background, executive summary is that sdbm is the only dbm using 
two files and we're stat:ing the wrong one.

Since this touches apr-util I will need some feedback on how we should 
fix this and not having it reoccur. As I see it we have three options:

1) Fix lookup_map() to stat both files if present. I don't like
    this simply because it's unneccessary since one file usually
    changes with each update and the other seldom changes.

2) Document the last file returned by apr_dbm_get_usednames* as
    the one to check for freshness (which is the current state of
    affairs) and fix cmd_rewritemap() to adhere to this.

3) Change and document the first file returned by
    apr_dbm_get_usednames* as the one to check.

Given that httpd tends to be built with system installed APR/APR-Util 
I'd say that option number two probably would be the best choice in 
order to be able to say that the issue is resolved. However, this 
requires that the suitable docco gets commited to apr-util so it's 
clear that it works (and will work) this way.

Option number three is probably the best way if we want to avoid 
unneccesary code just because the name of the file to check for 
freshness is returned in varying arguments...

I realize that this is as much an apr-util question as an httpd 
question, but I trust those involved in both camps to raise the 
issue with the apr-util crowd if needed.

Thoughts?

/Nikke
-- 
-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
  Niklas Edmundsson, Admin @ {acc,hpc2n}.umu.se      |     nikke@acc.umu.se
---------------------------------------------------------------------------
  MULTITASKING:  Locking up more than 1 app @ a time.
=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=

Re: PR41190 - which fix should we use

[Ruediger Pluem <rp...@apache.org>]

On 01/12/2008 04:27 PM, Niklas Edmundsson wrote:
> 
> Hi!
> 
> Since I no longer have an excuse I thought I should have a go at fixing
> PR41190, mod_rewrite stat:ing the wrong file when using sdbm maps.
> 
> Read http://issues.apache.org/bugzilla/show_bug.cgi?id=41190 for the
> background, executive summary is that sdbm is the only dbm using two
> files and we're stat:ing the wrong one.
> 
> Since this touches apr-util I will need some feedback on how we should
> fix this and not having it reoccur. As I see it we have three options:

To be honest I don't really like any of the three options, but I see no
real alternatives to them. More details below:

> 
> 1) Fix lookup_map() to stat both files if present. I don't like
>    this simply because it's unneccessary since one file usually
>    changes with each update and the other seldom changes.

I don't like it for same reason as you, but it has the advantage that is
does not need any new API guarantee on apr-util side.

> 
> 2) Document the last file returned by apr_dbm_get_usednames* as
>    the one to check for freshness (which is the current state of
>    affairs) and fix cmd_rewritemap() to adhere to this.

There are two points here:

1. I think it is no very intuitive to have the second filename be the
   one to check for freshness. It forces all users of the apr-util libary
   to implement a check whether the second filename is non NULL and check
   it in this case, but check the first one in the other case.

2. It is not clear to me if the current versioning rules or APR permit to
   add a new API guarantee to stable branches (and documenting it in fact
   creates a new API guarantee)

> 
> 3) Change and document the first file returned by
>    apr_dbm_get_usednames* as the one to check.

This sounds nice from the aspect of logic. We still have the same possible problem
with a new API guarantee as above.
While we do not break any API (there was no API guarantee before), we might
break applications that rely on the previous behaviour.

So I would propose the following:

1. Fix the httpd code to check the second file for freshness if present.
2. Move the apr-util discussion to the apr list and wait for the results
   of the discussion.
3. Possibly adjust the httpd code again depending on the decision made regarding
   apr-util and the apr list. This may result in different solutions for 2.0.x / 2.2.x
   and trunk because of the versioning rules I talked about above. 2.0.x / 2.2.x use
   stable branches of apr / apr-util while trunk requires apr / apr-util trunk.

Regards

Rüdiger