You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@apr.apache.org by fu...@apache.org on 2007/07/04 02:36:37 UTC

svn commit: r553027 - in /apr/apr-util/trunk: dbd/apr_dbd.c include/apu.h.in include/apu.hnw include/apu.hw

Author: fuankg
Date: Tue Jul  3 17:36:35 2007
New Revision: 553027

URL: http://svn.apache.org/viewvc?view=rev&rev=553027
Log:
added define APU_DBD_DRIVER_FMT to apu.h which sets the driver format to build the DSO name;
removed the ifdefs from apr_dbd.c; used apr_snprintf() also for symbol.

Modified:
    apr/apr-util/trunk/dbd/apr_dbd.c
    apr/apr-util/trunk/include/apu.h.in
    apr/apr-util/trunk/include/apu.hnw
    apr/apr-util/trunk/include/apu.hw

Modified: apr/apr-util/trunk/dbd/apr_dbd.c
URL: http://svn.apache.org/viewvc/apr/apr-util/trunk/dbd/apr_dbd.c?view=diff&rev=553027&r1=553026&r2=553027
==============================================================================
--- apr/apr-util/trunk/dbd/apr_dbd.c (original)
+++ apr/apr-util/trunk/dbd/apr_dbd.c Tue Jul  3 17:36:35 2007
@@ -150,19 +150,13 @@
     /* The driver DSO must have exactly the same lifetime as the
      * drivers hash table; ignore the passed-in pool */
     pool = apr_hash_pool_get(drivers);
-
-#ifdef WIN32
-    sprintf(path, "apr_dbd_%s.dll", name);
-#elif defined(NETWARE)
-    apr_snprintf(path, sizeof path, "dbd%s.nlm", name);
-#else
-    apr_snprintf(path, sizeof path, APU_DSO_LIBDIR "/apr_dbd_%s.so", name);
-#endif
+    /* APU_DBD_DRIVER_FMT is defined in apu.h */
+    apr_snprintf(path, sizeof path, APU_DBD_DRIVER_FMT, name);
     rv = apr_dso_load(&dlhandle, path, pool);
     if (rv != APR_SUCCESS) { /* APR_EDSOOPEN */
         goto unlock;
     }
-    sprintf(path, "apr_dbd_%s_driver", name);
+    apr_snprintf(path, sizeof path, "apr_dbd_%s_driver", name);
     rv = apr_dso_sym(&symbol, dlhandle, path);
     if (rv != APR_SUCCESS) { /* APR_ESYMNOTFOUND */
         apr_dso_unload(dlhandle);

Modified: apr/apr-util/trunk/include/apu.h.in
URL: http://svn.apache.org/viewvc/apr/apr-util/trunk/include/apu.h.in?view=diff&rev=553027&r1=553026&r2=553027
==============================================================================
--- apr/apr-util/trunk/include/apu.h.in (original)
+++ apr/apr-util/trunk/include/apu.h.in Tue Jul  3 17:36:35 2007
@@ -88,5 +88,7 @@
 #define APU_HAVE_ICONV         @have_iconv@
 #define APR_HAS_XLATE          (APU_HAVE_APR_ICONV || APU_HAVE_ICONV)
 
+#define APU_DBD_DRIVER_FMT APU_DSO_LIBDIR "/apr_dbd_%s.so"
+
 #endif /* APU_H */
 /** @} */

Modified: apr/apr-util/trunk/include/apu.hnw
URL: http://svn.apache.org/viewvc/apr/apr-util/trunk/include/apu.hnw?view=diff&rev=553027&r1=553026&r2=553027
==============================================================================
--- apr/apr-util/trunk/include/apu.hnw (original)
+++ apr/apr-util/trunk/include/apu.hnw Tue Jul  3 17:36:35 2007
@@ -82,5 +82,7 @@
 #define APU_HAVE_ICONV         1
 #define APR_HAS_XLATE          (APU_HAVE_APR_ICONV || APU_HAVE_ICONV)
 
+#define APU_DBD_DRIVER_FMT "dbd%s.nlm"
+
 #endif /* APU_H */
 /** @} */

Modified: apr/apr-util/trunk/include/apu.hw
URL: http://svn.apache.org/viewvc/apr/apr-util/trunk/include/apu.hw?view=diff&rev=553027&r1=553026&r2=553027
==============================================================================
--- apr/apr-util/trunk/include/apu.hw (original)
+++ apr/apr-util/trunk/include/apu.hw Tue Jul  3 17:36:35 2007
@@ -109,5 +109,7 @@
 #define APU_HAVE_SQLITE3    0
 #endif
 
+#define APU_DBD_DRIVER_FMT "apr_dbd_%s.dll"
+
 #endif /* APU_H */
 #endif /* WIN32 */



Re: svn commit: r553027 - in /apr/apr-util/trunk: dbd/apr_dbd.c include/apu.h.in include/apu.hnw include/apu.hw

Posted by Guenter Knauf <fu...@apache.org>.
Hi Martin,
>> sure it can also go into apu_config.h - but this needs then some more
>> changes:
>> - apr_dbd.h needs to include apu_config.h

> .... which is explicitly "private/apu_config.h" and should
>      therefore IMO not be used from outside.

>> - create a netware-own apu_config.hnw (not a problem)
>> - tell apr-util/configure.in to add this define (I would need some help
>> from you for that)

> ... the latter is probably the best approach.
can you please confirm if this will work for Unix/Linux:

--- configure.in.orig	Wed Jun 27 14:53:14 2007
+++ configure.in	Thu Jul 05 15:42:26 2007
@@ -173,6 +173,8 @@
 
 # Set up destination directory for DSOs.
 APU_DSO_LIBDIR="\${libdir}/apr-util-${APRUTIL_MAJOR_VERSION}"
+# Set up DBD driver format to load and print errors for driver DSOs.
+APU_DBD_DRIVER_FMT="\${libdir}/apr-util-${APRUTIL_MAJOR_VERSION}/apr_dbd_%s.so"
 # Set APU_HAVE_MODULES appropriately for the Makefile
 if test -n "$APU_MODULES"; then 
    APU_HAVE_MODULES=yes

then I would include apu_config.h from apr_dbd.h and it should work again:

--- apr_dbd.h.orig	Wed Jun 27 14:53:00 2007
+++ apr_dbd.h	Thu Jul 05 15:37:10 2007
@@ -22,6 +22,7 @@
 #define APR_DBD_H
 
 #include "apu.h"
+#include "apu_config.h"
 #include "apr_pools.h"
 
 #ifdef __cplusplus

dont know though if then an additional include path to include/private somewhere is needed....

thanks, Guenter.



Re: svn commit: r553027 - in /apr/apr-util/trunk: dbd/apr_dbd.c include/apu.h.in include/apu.hnw include/apu.hw

Posted by Martin Kraemer <ma...@apache.org>.
On Thu, Jul 05, 2007 at 03:45:42PM +0200, Guenter Knauf wrote:
> can you please confirm if this will work for Unix/Linux:

In file included from exports.c:51:
/home/martin/apachen/X/httpd-2.3/srclib/apr-util/include/apr_dbd.h:25: apu_config.h: No such file or directory

> dont know though if then an additional include path to include/private somewhere is needed....

Yes, but that is exactly the point: the "private" interface means
that "private/apu_config.h" is taboo for httpd (only kosher for
apr-util itself)

   Martin
-- 
<Ma...@Fujitsu-Siemens.com>        |     Fujitsu Siemens
http://www.fujitsu-siemens.com/imprint.html | 81730  Munich,  Germany

Re: svn commit: r553027 - in /apr/apr-util/trunk: dbd/apr_dbd.c include/apu.h.in include/apu.hnw include/apu.hw

Posted by Guenter Knauf <fu...@apache.org>.
Hi Martin,
>> sure it can also go into apu_config.h - but this needs then some more
>> changes:
>> - apr_dbd.h needs to include apu_config.h

> .... which is explicitly "private/apu_config.h" and should
>      therefore IMO not be used from outside.

>> - create a netware-own apu_config.hnw (not a problem)
>> - tell apr-util/configure.in to add this define (I would need some help
>> from you for that)

> ... the latter is probably the best approach.
can you please confirm if this will work for Unix/Linux:

--- configure.in.orig	Wed Jun 27 14:53:14 2007
+++ configure.in	Thu Jul 05 15:42:26 2007
@@ -173,6 +173,8 @@
 
 # Set up destination directory for DSOs.
 APU_DSO_LIBDIR="\${libdir}/apr-util-${APRUTIL_MAJOR_VERSION}"
+# Set up DBD driver format to load and print errors for driver DSOs.
+APU_DBD_DRIVER_FMT="\${libdir}/apr-util-${APRUTIL_MAJOR_VERSION}/apr_dbd_%s.so"
 # Set APU_HAVE_MODULES appropriately for the Makefile
 if test -n "$APU_MODULES"; then 
    APU_HAVE_MODULES=yes

then I would include apu_config.h from apr_dbd.h and it should work again:

--- apr_dbd.h.orig	Wed Jun 27 14:53:00 2007
+++ apr_dbd.h	Thu Jul 05 15:37:10 2007
@@ -22,6 +22,7 @@
 #define APR_DBD_H
 
 #include "apu.h"
+#include "apu_config.h"
 #include "apr_pools.h"
 
 #ifdef __cplusplus

dont know though if then an additional include path to include/private somewhere is needed....

thanks, Guenter.



Re: svn commit: r553027 - in /apr/apr-util/trunk: dbd/apr_dbd.c include/apu.h.in include/apu.hnw include/apu.hw

Posted by Martin Kraemer <ma...@apache.org>.
On Wed, Jul 04, 2007 at 01:37:08PM +0200, Guenter Knauf wrote:
> >> added define APU_DBD_DRIVER_FMT to apu.h which sets the driver format to
> >> build the DSO name;
> >> removed the ifdefs from apr_dbd.c; used apr_snprintf() also for symbol.
> 
> > Why does this need to be part of the public API?  (i.e. why isn't it
> > private to apr_dbd.c, or in apu_config.h?)
> I had the impression that the apu.h header is there for platform-dependent defines;
> and I use it from mod_dbd.c which only includes apu.h (via apr_dbd.h):
> http://svn.apache.org/viewvc?view=rev&revision=553031
> sure it can also go into apu_config.h - but this needs then some more changes:
> - apr_dbd.h needs to include apu_config.h

.... which is explicitly "private/apu_config.h" and should
     therefore IMO not be used from outside.

> - create a netware-own apu_config.hnw (not a problem)
> - tell apr-util/configure.in to add this define (I would need some help from you for that)

... the latter is probably the best approach.


Please revert this patch for now, or find a different solution --
currently httpd-2.3.x doesn't even compile on unix because
of this patch:
   mod_dbd.c: In function `dbd_param':
   mod_dbd.c:173: error: parse error before "APU_DSO_LIBDIR"
because APU_DBD_DRIVER_FMT uses the (private) define APU_DSO_LIBDIR
which is not visible to mod_dbd.

Of course it is debatable whether apr-util should make the
define APU_DBD_DRIVER_FMT public if it results in an undefined symbol
when used.....

IMHO the define for APU_DBD_DRIVER_FMT should either go into 
private/apu_config.h (where APU_DSO_LIBDIR is also defined), or
the define for APU_DSO_LIBDIR should be pulled in to make it public.

   Martin
-- 
<Ma...@Fujitsu-Siemens.com>        |     Fujitsu Siemens
http://www.fujitsu-siemens.com/imprint.html | 81730  Munich,  Germany

Re: svn commit: r553027 - in /apr/apr-util/trunk: dbd/apr_dbd.c include/apu.h.in include/apu.hnw include/apu.hw

Posted by Guenter Knauf <fu...@apache.org>.
Hi Joe,
> On Wed, Jul 04, 2007 at 12:36:37AM -0000, fuankg@apache.org wrote:
>> Author: fuankg
>> Date: Tue Jul  3 17:36:35 2007
>> New Revision: 553027
>>
>> URL: http://svn.apache.org/viewvc?view=rev&rev=553027
>> Log:
>> added define APU_DBD_DRIVER_FMT to apu.h which sets the driver format to
>> build the DSO name;
>> removed the ifdefs from apr_dbd.c; used apr_snprintf() also for symbol.

> Why does this need to be part of the public API?  (i.e. why isn't it
> private to apr_dbd.c, or in apu_config.h?)
I had the impression that the apu.h header is there for platform-dependent defines;
and I use it from mod_dbd.c which only includes apu.h (via apr_dbd.h):
http://svn.apache.org/viewvc?view=rev&revision=553031
sure it can also go into apu_config.h - but this needs then some more changes:
- apr_dbd.h needs to include apu_config.h
- create a netware-own apu_config.hnw (not a problem)
- tell apr-util/configure.in to add this define (I would need some help from you for that)

Guenter.



Re: svn commit: r553027 - in /apr/apr-util/trunk: dbd/apr_dbd.c include/apu.h.in include/apu.hnw include/apu.hw

Posted by Joe Orton <jo...@redhat.com>.
On Wed, Jul 04, 2007 at 12:36:37AM -0000, fuankg@apache.org wrote:
> Author: fuankg
> Date: Tue Jul  3 17:36:35 2007
> New Revision: 553027
> 
> URL: http://svn.apache.org/viewvc?view=rev&rev=553027
> Log:
> added define APU_DBD_DRIVER_FMT to apu.h which sets the driver format to build the DSO name;
> removed the ifdefs from apr_dbd.c; used apr_snprintf() also for symbol.

Why does this need to be part of the public API?  (i.e. why isn't it 
private to apr_dbd.c, or in apu_config.h?)

joe