Last week we have discussed on the modperl list a bug in document_root, where when setting it, we were just pointing the s->document_root to the PV slot in the scalar, which was obviously wrong. So it needed to be copied via pool and restored to the original when that pool went out of scope (as Joe has suggested).

As at appears there is a bunch of other methods which suffer from a similar but not indenical problem. Take for example $s->server_admin.

char *
server_admin(obj, val=NULL)
    Apache::ServerRec obj
    char * val
[...]
    RETVAL = (char *) obj->server_admin;

    if (items > 1) {
         MP_CROAK_IF_THREADS_STARTED("setting server_admin");
         obj->server_admin = (char *) val;
    }

Here the problem is the same:

        obj->server_admin = (char *) val;

is wrong.

But in this case we can't allocate it from the pool, since the object that we have is server object, and if we allocate from it, we will get a continuous memory leak. The only solution I can see is the one suggested earlier by Geoff, which comes from mp1:

  SV *server_admin = perl_get_sv("Apache::ServerRec::ServerAdmin", TRUE);
  sv_setsv(server_admin, ST(1));
  obj->server_admin = SvPVX(serever_admin);

i.e. we create a superficial perl variable to make sure that the value will stick around if the original scalar is modified or destroyed.

Anybody sees a better solution?

If we go with this one, should we use some obscure name space for this kind of stashing? We don't really want to support more APIs, do we? and using the "Apache::ServerRec::ServerAdmin" is something that people will discover and certainly use. Which of course could be a good thing too, as it'll be faster, but on the other hand it won't be set unless a user has modified $s->server_admin. That's why I think it should be hidden into something like: $_modperl_private::apache_server_admin

Ideas?

Finally if we go with that solution, should we use it for $r->document_root as well, so it'll behave more like mp1 did? in other thread we agreed that it's a goodness to restore it at the end of each request. I think we should stick to that solution.

--
__________________________________________________________________
Stas Bekman            JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/     mod_perl Guide ---> http://perl.apache.org
mailto:[EMAIL PROTECTED] http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com

---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]



Reply via email to