apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Guenter Knauf <fua...@apache.org>
Subject Re: APU_DBD_DRIVER_FMT should be private
Date Tue, 10 Jul 2007 12:03:53 GMT

> In the discussion around the error reporting of dlopen() errors when
> loading APR database drivers, I have understood that

> * currently, only the fact that a driver failed to load can be
>   reported back to the caller

> * the caller gets APR_EDSOOPEN when dynamic loading failed, but no clue
>   about the name of the file which caused the error.

> At the moment, APU_DBD_DRIVER_FMT describes the format that is used
> internally in apr_dbd.c to build the name of the shared object, from
> the value of the APU_DSO_LIBDIR macro (which is apu-private from
> apr-util/include/private/apu_config.h) and the name of the driver,
> by using:
>   #define APU_DBD_DRIVER_FMT  APU_DSO_LIBDIR "/apr_dbd_%s.so"
>   ...
>   apr_snprintf(path, sizeof path, APU_DBD_DRIVER_FMT, name);

> 1)
>   This is going to break badly if APR is installed into a directory
>   that is called, say,
>      /my/na%sty/apr-lib/
>   because then the two %s are both interpreted as string expansion
>   format chars, and apr will probably dump core.

>   Suggestion:
>     use
>      #define APU_DBD_DRIVER_FMT "%s/apr_dbd_%s.so"
>     and
>      apr_snprintf(path, sizeof path, APU_DBD_DRIVER_FMT, APU_DSO_LIBDIR,
>      name);
>   this should protect against directories with '%' characters.

> 2) the APU_DBD_DRIVER_FMT should IMO not be public at all, but
>   should be even more local (local to apr_dbd.c) -- its usage semantics
>   are highly specific to apr_dbd.c

> 3) For finding out the name that caused dynamic loading to fail, the
>    interface of the function doing the loading --apr_dbd_get_driver()--
>    should either be extended to allow for returning the absolute path
>    of the failing shared lib,
>    or a new function should be added that returns the name of the
>    shared object in question, when given the same driver name as
>    apr_dbd_get_driver(), something like

>     /* Get the absolute path of the driver's shared object */
>     apr_status_t apr_dbd_get_driver_dso_path(apr_pool_t *pool,
>                                        const char *name,
>                                        char **returned_name);

>    that fails with APR_ENOTIMPL if !defined(APU_DSO_BUILD)

> What do you think?
I find this is a good approach if we really need APU_DSO_BUILD.

When I introduced the APU_DBD_DRIVER_FMT I thought that I could make things simpler;
but from your previous and recent comments I see now that there is not such a simple solution
also I'm asking me why we need to provide a hardcoded APU_DSO_LIBDIR at all?
Another solution which I would find easiest would be if we would just configure the full driver
path/name in mod_dbd instead of using a driver alias only; this would stop the ping-pong we
would have to do else:
- send the driver alias to apr_dbd
- apr_dbd creates a not at runtime changeable full path from that and tries to load the driver
- on load failure we would have to ask apr_dbd to give us the full driver path back in order
to tell the user where we expected the driver

if we would configure the full driver path/name in mod_dbd the user would all the time know
(since he configured self) where APU expects the driver + mod_dbd could provide a proper error
message on load failure; we would then load the DBD driver just as we do with the other (httpd)
DSO modules = specify a full or relative path (relative to server root). This way both APU_DBD_DRIVER_FMT
and APU_DSO_LIBDIR would become obsolete (unless APU_DSO_LIBDIR is also used for other loads
which I'm not aware). I beleive that would provide most flexibility.

If there are no more comments or advises I will later this evening revert my changes and remove
and restore the way it was before where apr_dbd.c has its ifdefs + change the apr_snprinf()
for Unix so as Martin suggested.

thanks, Guenter.

View raw message