Re: dbd pgsql autotools patch
On Mon, 2008-05-19 at 20:05 -0400, Bob Rossi wrote: If you make the change, I'll test out the latest snapshots and all your fixes for me. Thanks so much Bojan! Done. And you are welcome. -- Bojan
Re: dbd pgsql autotools patch
On Mon, 2008-05-19 at 20:47 -0400, Bob Rossi wrote: This is almost correct. It's simply missing the -L to the postgresql/lib directory. My program failed to link because it couldn't find -lpgport which lives there. If fact, PostgreSQL manual (http://www.postgresql.org/docs/8.3/static/libpq-build.html) confirms that we should be using just --libdir. It should be committed now. Let me know... -- Bojan
Re: dbd pgsql autotools patch
On Tue, 2008-05-20 at 11:21 +1000, Bojan Smojver wrote: If fact, PostgreSQL manual (http://www.postgresql.org/docs/8.3/static/libpq-build.html) confirms that we should be using just --libdir. BTW, if --libdir alone doesn't cut it for you for some reason, I'll add --libs back too. But, if it does, I prefer to keep it simple. -- Bojan
Re: sqlite3 dbd provider question
On Mon, 2008-05-19 at 18:48 -0700, Chris Darroch wrote: Personally, I'd suggest just removing it from trunk too ... I'm not sure exactly how a global cache could be made to work across multiple databases, since they might be of different versions, etc. It's not likely to be trivial, I would think, and I'm not sure what the upside is. I agree. A big complication in order to shave a bit of runtime for small number of users, if any. So, anyway, here's a patch against trunk ... it should apply cleanly against 1.3 as well, I believe. If you're not applying this to trunk as well, please apply the previous patch, which at least moves the apr_dbd_mutex_unlock() calls into matching #ifdefs. Thanks, I'll let Bill commit this, given that he was the one handling the whole thing. Thanks for the patch Chris! -- Bojan
Re: dbd pgsql autotools patch
Here is a patch I've been toying with. Essentially, it does these things: - cleans up PostgreSQL detection a bit more - uses APRUTIL_PRIV_INCLUDES for all CPPFLAGS driver stuff (i.e. we don't need to expose that to users of APU) - is careful not to place LDFLAGS of drivers into APRUTIL_LDFLAGS (i.e. again, these are used for driver link, nothing else) Let me know if you think this is better than before... -- Bojan Index: build/dbd.m4 === --- build/dbd.m4 (revision 658076) +++ build/dbd.m4 (working copy) @@ -44,6 +44,9 @@ if test $apu_have_pgsql = 0; then AC_CHECK_HEADERS(postgresql/libpq-fe.h, AC_CHECK_LIB(pq, PQsendQueryPrepared, [apu_have_pgsql=1])) fi + if test $apu_have_pgsql != 0 test x$PGSQL_CONFIG != 'x'; then +APR_ADDTO(APRUTIL_PRIV_INCLUDES, [$pgsql_CPPFLAGS]) + fi elif test $withval = no; then : else @@ -61,17 +64,12 @@ AC_MSG_NOTICE(checking for pgsql in $withval) AC_CHECK_HEADERS(libpq-fe.h, AC_CHECK_LIB(pq, PQsendQueryPrepared, [apu_have_pgsql=1])) - if test $apu_have_pgsql != 0; then -APR_ADDTO(APRUTIL_LDFLAGS, [-L$withval/lib]) -APR_ADDTO(APRUTIL_INCLUDES, [-I$withval/include]) - fi if test $apu_have_pgsql != 1; then AC_CHECK_HEADERS(postgresql/libpq-fe.h, AC_CHECK_LIB(pq, PQsendQueryPrepared, [apu_have_pgsql=1])) -if test $apu_have_pgsql != 0; then - APR_ADDTO(APRUTIL_INCLUDES, [-I$withval/include/postgresql]) - APR_ADDTO(APRUTIL_LDFLAGS, [-L$withval/lib]) -fi fi + if test $apu_have_pgsql != 0; then +APR_ADDTO(APRUTIL_PRIV_INCLUDES, [$pgsql_CPPFLAGS]) + fi fi ], [ AC_PATH_PROG([PGSQL_CONFIG],[pg_config]) @@ -87,12 +85,15 @@ if test $apu_have_pgsql = 0; then AC_CHECK_HEADERS(postgresql/libpq-fe.h, AC_CHECK_LIB(pq, PQsendQueryPrepared, [apu_have_pgsql=1])) fi +if test $apu_have_pgsql != 0 test x$PGSQL_CONFIG != 'x'; then + APR_ADDTO(APRUTIL_PRIV_INCLUDES, [$pgsql_CPPFLAGS]) +fi ]) AC_SUBST(apu_have_pgsql) dnl Since we have already done the AC_CHECK_LIB tests, if we have it, dnl we know the library is there. if test $apu_have_pgsql = 1; then -LDADD_dbd_pgsql=-lpq +LDADD_dbd_pgsql=$pgsql_LDFLAGS -lpq fi AC_SUBST(LDADD_dbd_pgsql) @@ -123,11 +124,10 @@ AC_CHECK_HEADERS(mysql.h, AC_CHECK_LIB(mysqlclient_r, mysql_init, [apu_have_mysql=1])) if test $apu_have_mysql = 0; then AC_CHECK_HEADERS(mysql/mysql.h, AC_CHECK_LIB(mysqlclient_r, mysql_init, [apu_have_mysql=1])) - else -if test x$MYSQL_CONFIG != 'x'; then - APR_ADDTO(APRUTIL_INCLUDES, [$mysql_CPPFLAGS]) -fi fi + if test $apu_have_mysql != 0 test x$MYSQL_CONFIG != 'x'; then +APR_ADDTO(APRUTIL_PRIV_INCLUDES, [$mysql_CPPFLAGS]) + fi elif test $withval = no; then : else @@ -145,16 +145,13 @@ AC_MSG_NOTICE(checking for mysql in $withval) AC_CHECK_HEADERS(mysql.h, AC_CHECK_LIB(mysqlclient_r, mysql_init, [apu_have_mysql=1])) - if test $apu_have_mysql != 0; then -APR_ADDTO(APRUTIL_INCLUDES, [$mysql_CPPFLAGS]) - fi if test $apu_have_mysql != 1; then AC_CHECK_HEADERS(mysql/mysql.h, AC_CHECK_LIB(mysqlclient_r, mysql_init, [apu_have_mysql=1])) -if test $apu_have_mysql != 0; then - APR_ADDTO(APRUTIL_INCLUDES, [-I$withval/include/mysql]) -fi fi + if test $apu_have_mysql != 0; then +APR_ADDTO(APRUTIL_PRIV_INCLUDES, [$mysql_CPPFLAGS]) + fi fi ]) @@ -195,8 +192,7 @@ AC_MSG_NOTICE(checking for sqlite3 in $withval) AC_CHECK_HEADERS(sqlite3.h, AC_CHECK_LIB(sqlite3, sqlite3_open, [apu_have_sqlite3=1])) if test $apu_have_sqlite3 != 0; then -APR_ADDTO(APRUTIL_LDFLAGS, [-L$withval/lib]) -APR_ADDTO(APRUTIL_INCLUDES, [-I$withval/include]) +APR_ADDTO(APRUTIL_PRIV_INCLUDES, [-I$withval/include]) fi fi ], [ @@ -208,7 +204,7 @@ dnl Since we have already done the AC_CHECK_LIB tests, if we have it, dnl we know the library is there. if test $apu_have_sqlite3 = 1; then -LDADD_dbd_sqlite3=-lsqlite3 +LDADD_dbd_sqlite3=$sqlite3_LDFLAGS -lsqlite3 fi AC_SUBST(LDADD_dbd_sqlite3) @@ -240,8 +236,7 @@ AC_MSG_NOTICE(checking for sqlite2 in $withval) AC_CHECK_HEADERS(sqlite.h, AC_CHECK_LIB(sqlite, sqlite_open, [apu_have_sqlite2=1])) if test $apu_have_sqlite2 != 0; then -APR_ADDTO(APRUTIL_LDFLAGS, [-L$withval/lib]) -APR_ADDTO(APRUTIL_INCLUDES, [-I$withval/include]) +APR_ADDTO(APRUTIL_PRIV_INCLUDES, [-I$withval/include]) fi fi ], [ @@ -253,7 +248,7 @@ dnl Since we have already done the AC_CHECK_LIB tests, if we have it, dnl we know the library is there. if test $apu_have_sqlite2 = 1; then -
Re: sqlite3 and postgresql
On Sat, 2008-05-17 at 20:43 -0400, Tom Donovan wrote: I would be happy to take care of it as soon as I receive commit access. I'm a new committer, and the instructions advised patience - so I expect this usually takes some time to get processed. I committed something that _may_ work in r657516. The caveat is that we rely here on both PostgreSQL and MySQL to not blow up in bad ways if they get rownum of -1 (i.e. if the user of our API passes in row 0). I did not test this at all, so feel free to do so. -- Bojan
Re: svn commit: r657516 - in /apr/apr-util/trunk/dbd: apr_dbd_mysql.c apr_dbd_pgsql.c
On Sun, 2008-05-18 at 11:24 +0200, Ruediger Pluem wrote: I guess this should now be 0? Depends, really. If we want things to break when users pass in 0, then it should be the way it is, provided MySQL returns an error when row it is given is -1. If we just want to not seek at all, then yes - we should make it 0. Given that we're changing the semantics here, we just need to pick one. I'm cool either way. -- Bojan
Re: svn commit: r657516 - in /apr/apr-util/trunk/dbd: apr_dbd_mysql.c apr_dbd_pgsql.c
On Sun, 2008-05-18 at 12:02 +0200, Ruediger Pluem wrote: Maybe we should return a driver independent error code in the case that the row number is 1. A MySQL specific error code might be somewhat confusing to the user as it might tell him that he used -1 as a row number. BTW: As far as I see the old behaviour was not to seek at all if the row number was 0. Was this intended? I think so. BTW, I think we'll have to do 0 as you suggested because we cast into (my_ulonglong), which if it gets value of -1 may become a really, really big number thus potentially screwing a lot of things and giving meaningless error messages back. Anyhow, I'll do the 0 now. -- Bojan
Re: sqlite3 and postgresql
On Sat, 2008-05-17 at 09:07 -0400, Bob Rossi wrote: I've been using sqlite3 dbd for some time now. I find that the function apr_dbd_get_row, http://apr.apache.org/docs/apr-util/trunk/group___a_p_r___util___d_b_d.html#gd4cdc5f4e8981b93f5a467a8c8a768f1 is 1 based. That is, 1 is the first row returned from a query. However, I just started using postgresql, and I find that apr_dbd_get_row is 0 based. That is, 0 is the first row returned from the query. This should now be fixed in both the trunk and 1.3.x branch. We really _do_ appreciate your testing and suggestions! -- Bojan
Re: upgrading from 2006 snapshot to apr-1.3.x
On Sat, 2008-05-17 at 06:56 -0400, Bob Rossi wrote: Huh, I build with --disable-shared --enable-static. Perhaps it doesn't work in this configuration? Possibly. Have you tried with --disable-dbd-dso? -- Bojan
Re: upgrading from 2006 snapshot to apr-1.3.x
On Sat, 2008-05-17 at 06:56 -0400, Bob Rossi wrote: char path[80]; Yeah, most likely should be APR_PATH_MAX+1, or something. -- Bojan
Re: upgrading from 2006 snapshot to apr-1.3.x
On Sat, 2008-05-17 at 07:21 -0400, Bob Rossi wrote: Just tried it, my tests pass now, which means this works. Should --disable-shared force --disable-dbd-dso? It does disable it - drivers don't work :-) (Sorry, couldn't resist to be a smartarse). Seriously, this could be done for convenience, but we may just mention it in README or some other doco. Also, the path issue is pretty serious imo. Yeah, I think we should make it bigger than 80. -- Bojan
Re: sqlite3 and postgresql
On Sat, 2008-05-17 at 10:03 -0400, Tom Donovan wrote: I think this is a valid bug. I agree. -- Bojan
Re: sqlite3 and postgresql
On Sat, 2008-05-17 at 18:29 -0500, William A. Rowe, Jr. wrote: +1 and I concur with Tom's suggestion w.r.t. a preferred row offset 1. Tom, do you want to fix this one or do you want me to do it? -- Bojan
Re: sqlite3 and postgresql
Quoting Bob Rossi [EMAIL PROTECTED]: Aren't you worried about how that will break all the existing programs out there using this interface? This is one of those damned if we fix it, damned if we don't things :-) That being said, this is clearly a bug, as APU's DBD is supposed to be database neutral as much a possible (i.e. you should be able to change the underlying driver and still have most of the stuff just work). For that reason, it should be fixed. In terms of strict binary compatibility, 1.2.x most likely cannot be changed here because we state here http://apr.apache.org/versioning.html#strategy that for the Patch Version: Changes to the API, to the signatures of public functions, or to the interpretation of function parameters is not allowed. AFAICT, Minor Versions do not have this particular requirement, so we should be OK to fix this in 1.3.x. PS. We already did similar changes to DBD functions in 1.3.x. -- Bojan
Re: postgres dbd_pgsql_open
On Fri, 2008-05-16 at 16:32 -0400, Bob Rossi wrote: I'm looking at dbd_pgsql_open. http://apr.apache.org/docs/apr-util/trunk/group___a_p_r___util___d_b_d.html#gbddb1fdcb2f8a5f5b83127485c78e8ae -- Bojan
Re: dbd pgsql autotools patch
On Fri, 2008-05-16 at 06:26 -0400, Bob Rossi wrote: configure:40279: gcc -o conftest -g -Wall -pthread -DLINUX=2 -D_REENTRANT -D_GNU_SOURCE conftest.c -lcrypt 5 configure:40285: $? = 0 configure:40313: result: -lcrypt So, the test for crypt actually succeeds. Maybe the whole thing is caused by the fact that we check for crypt after we did DBD stuff. Can you tell me if the attached patch works for you? PS. The patch is against 1.3.x, but it should apply against 1.2.x as well, with some fuzz. -- Bojan Index: configure.in === --- configure.in (revision 655406) +++ configure.in (working copy) @@ -142,6 +142,21 @@ ]) AC_SUBST(APR_ICONV_DIR) +AC_SEARCH_LIBS(crypt, crypt ufc) +AC_MSG_CHECKING(if system crypt() function is threadsafe) +if test x$apu_crypt_threadsafe = x1; then + AC_DEFINE(APU_CRYPT_THREADSAFE, 1, [Define if the system crypt() function is threadsafe]) + msg=yes +else + msg=no +fi +AC_MSG_RESULT([$msg]) + +AC_CHECK_FUNCS(crypt_r, [ crypt_r=1 ], [ crypt_r=0 ]) +if test $crypt_r = 1; then + APU_CHECK_CRYPT_R_STYLE +fi + dnl Find LDAP library dnl Determine what DBM backend type to use. dnl Find Expat @@ -159,21 +174,6 @@ APU_FIND_EXPAT APU_FIND_ICONV -AC_SEARCH_LIBS(crypt, crypt ufc) -AC_MSG_CHECKING(if system crypt() function is threadsafe) -if test x$apu_crypt_threadsafe = x1; then - AC_DEFINE(APU_CRYPT_THREADSAFE, 1, [Define if the system crypt() function is threadsafe]) - msg=yes -else - msg=no -fi -AC_MSG_RESULT([$msg]) - -AC_CHECK_FUNCS(crypt_r, [ crypt_r=1 ], [ crypt_r=0 ]) -if test $crypt_r = 1; then - APU_CHECK_CRYPT_R_STYLE -fi - so_ext=$APR_SO_EXT lib_target=$APR_LIB_TARGET AC_SUBST(so_ext)
Re: postgres dbd_pgsql_open
On Fri, 2008-05-16 at 20:12 -0400, Bob Rossi wrote: I'm glad to see someone beat me to it! Glad to be of service :-) -- Bojan
Re: dbd pgsql autotools patch
On Fri, 2008-05-16 at 20:33 -0400, Bob Rossi wrote: Yes, that worked! Cool. Thanks for testing! Do you think this will get into the 1.3 snapshot? Well, I think it should go in, hence I'll commit to trunk and ask other committers to comment on in before backporting to 1.3.x/1.2.x. We need to be careful not to break other things now... -- Bojan
Re: dbd pgsql autotools patch
On Fri, 2008-05-16 at 19:42 -0500, William A. Rowe, Jr. wrote: Actually that's half-assed. We have to back out the changes that the DBD detection causes for LDFLAGS and LIBS, etc, in order to ensure that later tests aren't bogus. We already did that, no? We do it like this in build/dbd.m4: old_cppflags=$CPPFLAGS old_ldflags=$LDFLAGS [...other stuff...] CPPFLAGS=$old_cppflags LDFLAGS=$old_ldflags This crypt business is not part of build/dbd.m4 - it is in configure.in and can potentially affect other things. -- Bojan
Re: dbd pgsql autotools patch
On Sat, 2008-05-17 at 10:43 +1000, Bojan Smojver wrote: ask other committers to comment Well, I didn't have to ask, did I? ;-) -- Bojan
Re: dbd pgsql autotools patch
On Fri, 2008-05-16 at 19:51 -0500, William A. Rowe, Jr. wrote: No but since you did, my +1. Thanks. BTW, there is one place in PostgreSQL detection where we don't return LDFLAGS and CPPFLAGS back to normal. I'll fix that too. -- Bojan
Re: dbd pgsql autotools patch
On Sat, 2008-05-17 at 10:53 +1000, Bojan Smojver wrote: BTW, there is one place in PostgreSQL detection where we don't return LDFLAGS and CPPFLAGS back to normal. I'll fix that too. Ignore this comment please - this is for the case we we don't fiddle with CPPFLAGS/LDFLAGS. -- Bojan
Re: upgrading from 2006 snapshot to apr-1.3.x
On Fri, 2008-05-16 at 22:43 -0400, Bob Rossi wrote: I just upgraded to the 1.3 svn snapshot, to test out the new functionality that was recently added. After I ran my unit tests I noticed that the function apr_dbd_init() was not working properly for me. Then I realized it's because this code, With 1.3.x and above, we build DBD modules as DSOs. Did they get built properly? -- Bojan
Re: dbd pgsql autotools patch
On Thu, 2008-05-15 at 09:08 -0400, Bob Rossi wrote: Attached is the patch that fixes the problem, what do you think? Yeah, we do this kind of thing for other databases already. I'll have a look at it today. -- Bojan
Re: dbd pgsql autotools patch
On Thu, 2008-05-15 at 09:08 -0400, Bob Rossi wrote: So, I get a link error, because it needs to link against -lcrypt. What kind of system do you have? Where does the -lcrypt live on your system? BTW, doing the pg_config --libs will not give us the right thing here. For instance, on my system, this returns: -lpgport -lxslt -lxml2 -lpam -lssl -lcrypto -lgssapi_krb5 -lz -lreadline -lcrypt -ldl -lm Many of these things are most definitely not needed for the client to be linked in (i.e. the --libs gives libraries used to build PostgreSQL, not ones required to link to pq). We have to find a better way to do this... -- Bojan
Re: dbd pgsql autotools patch
On Thu, 2008-05-15 at 23:18 -0400, Bob Rossi wrote: Ubuntu 7.04, /lib/libcrypt-2.5.so OK. OK, perhaps we should just add -lcrypt to the pg line? I could check to see if this works on my system if you think this is a good idea. Hmm, not sure if hardcoding here is the right thing to do. I don't see how that got omitted from the set of libraries that are required. What do you get when you run apr-1-config --libs (make sure you run apr-1-config of the one you used to build apr-util)? -- Bojan
Re: apr_reslist semantics
On Mon, 2008-05-12 at 17:38 -0500, William A. Rowe, Jr. wrote: Do folks feel we should release an apr+apu this friday? If 1.3.0 has settled in, then that is the release. If not, a final 1.2.x release instead, in the interim? Given Joe's latest comments regarding --with-ldap, I would be then inclined to say 1.3.0. Unless there are some other major dramas that cannot be fixed before then... -- Bojan
Re: How to modify Makefile for apr_pools.c ?
On Wed, 2008-05-14 at 10:42 +0800, zengwm wrote: So making apr_pools.c and linking may require adding some arguments to the command line to link the library, e.g. -L and -l.. You can try like this: LDFLAGS=your_flags_here ./configure your_options_here The run make etc. -- Bojan
Re: apr_reslist semantics
On Mon, 2008-05-12 at 16:10 -0500, William A. Rowe, Jr. wrote: Like Roy, I'm frustrated with aprutil's dependency madness, and I'm working right now on abstracting out apu-util into a loadable module. At this point a --with-ldap build on linux FC8 is producing this pile of crap as bindings for those applications with no use for ldap; +libldap-2.3.so.0 +liblber-2.3.so.0 +libresolv.so.2 +libsasl2.so.2 +libssl.so.6 +libcrypto.so.6 +libgssapi_krb5.so.2 +libkrb5.so.3 +libcom_err.so.2 +libk5crypto.so.3 +libz.so.1 +libkrb5support.so.0 +libkeyutils.so.1 +libselinux.so.1 Aren't these coming from OpenLDAP: --- $ ldd /usr/lib/libldap.so linux-gate.so.1 = (0x0011) liblber-2.3.so.0 = /usr/lib/liblber-2.3.so.0 (0x00c83000) libresolv.so.2 = /lib/libresolv.so.2 (0x005da000) libsasl2.so.2 = /usr/lib/libsasl2.so.2 (0x02d7f000) libssl.so.6 = /lib/libssl.so.6 (0x004d1000) libcrypto.so.6 = /lib/libcrypto.so.6 (0x02fb) libc.so.6 = /lib/libc.so.6 (0x00744000) libdl.so.2 = /lib/libdl.so.2 (0x008ca000) libcrypt.so.1 = /lib/libcrypt.so.1 (0x03c7f000) libgssapi_krb5.so.2 = /usr/lib/libgssapi_krb5.so.2 (0x00cd5000) libkrb5.so.3 = /usr/lib/libkrb5.so.3 (0x00d54000) libcom_err.so.2 = /lib/libcom_err.so.2 (0x00101000) libk5crypto.so.3 = /usr/lib/libk5crypto.so.3 (0x00cad000) libz.so.1 = /lib/libz.so.1 (0x008ec000) /lib/ld-linux.so.2 (0x00721000) libkrb5support.so.0 = /usr/lib/libkrb5support.so.0 (0x00b34000) libkeyutils.so.1 = /lib/libkeyutils.so.1 (0x006e4000) libselinux.so.1 = /lib/libselinux.so.1 (0x00186000) --- BTW, if you do link to to OpenLDAP on F8, all these things do make sense to OpenLDAP - they are not useless. Or did I miss something? -- Bojan
Re: apr_reslist semantics
On Mon, 2008-05-12 at 16:39 -0500, William A. Rowe, Jr. wrote: Which is my point, a loadable apr_ldap.so stub which is loaded at runtime by applications which need it is the simplest solution. With the existing apr_ldap_init function, this should all be transparent. Ah, sorry :-( I thought you wanted to avoid linking to these at all - which of course cannot be done with F8's OpenLDAP. Please ignore me - ENOCOFFEE :-) -- Bojan
Re: testreslist failure?
On Mon, 2008-05-12 at 12:45 -0500, William A. Rowe, Jr. wrote: Is anyone else seeing testreslist spin for an excessive amount of time? This is on Linux 2.6.24.5-85.fc8 #1 SMP Sat Apr 19 12:39:34 EDT 2008 i686 i686 i386 GNU/Linux Ditto here (i.e. just confirming your result). -- Bojan
Re: DBD driver license issues (must default to --without-driver)
On Sun, 2008-05-11 at 21:35 +0200, Ruediger Pluem wrote: This already works if you compile apr-util (trunk / 1.3) with --enable-dbd-dso Maybe that should be the default, combined with disabling of MySQL/Oracle drivers by default due to possible downstream licensing issues? -- Bojan
Re: DBD driver license issues (must default to --without-driver)
On Sun, 2008-05-11 at 18:36 -0500, William A. Rowe, Jr. wrote: Are there concerns that this is either insufficiently portable or stable enough to serve as our 1.3 default? Not sure about portability, but it is as stable as linking the drivers directly. -- Bojan
Re: DBD driver license issues (must default to --without-driver)
On Sun, 2008-05-11 at 20:35 -0400, Tom Donovan wrote: Maybe that should be the default, combined with disabling of MySQL/Oracle drivers by default due to possible downstream licensing issues? I agree that --enable-dbd-dso should be the default (presuming the platform supports dso), but not for licensing reasons. I didn't say that --enable-dbd-dso should be default for licensing reasons. I said it should be _combined_ with disabling of MySQL/Oracle drivers by default, which is for licensing reasons. -- Bojan
Re: DBD driver license issues (must default to --without-driver)
On Sun, 2008-05-11 at 21:08 -0500, William A. Rowe, Jr. wrote: Yes - drop any automatically detected dbd provider who's license is more restrictive than APR's. If someone such as a linux distro (GPL) wants to turn back on the mysql dbd, this is their prerogative. And enable dbd-dso by default even without one provider. In r655174 and r655403 of the trunk now. -- Bojan
Re: DBD driver license issues (must default to --without-driver)
On Mon, 2008-05-12 at 12:55 +1000, Bojan Smojver wrote: In r655174 and r655403 of the trunk now. Backported to 1.3.x in r655404. -- Bojan
Re: Why the 'size' is aligned twice in apr_pCalloc()
On Mon, 2008-05-12 at 11:19 +0800, zengwm wrote: apr_pCalloc() calls apr_palloc() to do the memory allocation in the pool. It aligns the 'size' parameter before calling apr_palloc(). And the size is aligned again in apr_palloc(). Why 'size' has to be aligned twice? Can the first alignment ( in apr_pCalloc() ) be omitted? I don't think omitting the first alignment will affect the memset() call later in apr_pCalloc(), because, from the users' point of view, it's guaranteed that all the memory he can see has been Zero-out. This may be an artifact of the old implementation of apr_pcalloc(), which did not call apr_palloc(). I'd be inclined to agree with your reasoning here, but because I didn't actually implement this function, I don't know if there could be some hidden reliance on the total aligned size being zeroed here. Anyone else cares to comment? -- Bojan
Re: DBD driver license issues (must default to --without-driver)
On Sat, 2008-05-10 at 13:58 -0700, Roy T. Fielding wrote: This is a showstopper. It must be fixed in 1.3.x before release. My fix is also incomplete: All of the other DBD drivers that do not have redistributable client libraries must also default off. Oracle is one for sure -- I am unaware of the terms for the other drivers. Quick summary: - MySQL: GPL licence, allows open source (including APR) to link to it; people shipping APR inside a proprietary piece of code may be in trouble for linking to GPL licensed MySQL, better turn it off by default - FreeTDS: LGPL licence, OK to link to by anyone - Oracle: proprietary licence, better stay clear to avoid getting downstream people in trouble by default - SQLite: public domain, OK to link by anyone - PostgreSQL: BSD licence, OK to link by anyone -- Bojan
Re: freezing 1.3 tonight
On Thu, 2008-05-01 at 21:25 +0100, Christopher Key wrote: I'm not sure whether this has been covered already, and whether it needs to go in during a major release, but is there any chance of adding apr_int8_t and apr_uint8_t typedefs? AFAICT, this was left off with Garrett asking for a bug to be filed. If you do that and attach the patch to it, Bill may be in a better position to roll with it. -- Bojan
Re: PR #44881
On Mon, 2008-04-28 at 17:33 -0500, William A. Rowe, Jr. wrote: Interesting thought, keep in mind the other half of the issue is the number of times we consume generate_random_bytes ourselves from other functions, you'll have to suggest which should be pseudo, which should be truly random and which should be configurable. Bill, are we doing something about this before 1.3.0 gets out? -- Bojan
Re: PR #44881
On Thu, 2008-05-01 at 18:05 -0500, William A. Rowe, Jr. wrote: So no, I would not change the manner that UUID's are generated to urandom. generate_random_bytes is defined to provide the greatest entropy we can obtain. It is not, after all, generate_psuedorandom_bytes. OK, thanks. I'll update the bug report. -- Bojan
Re: PR #44881
On Fri, 2008-05-02 at 02:56 +0300, Lucian Adrian Grijincu wrote: 1. Some function in APR's uuid generator falls back to rand(3) if apr_generate_random_bytes returns an error. ... 2. E2fsprogs on which other major open source UUID generators are supposed to be based on (at least according to http://en.wikipedia.org/wiki/UUID) tries to open /dev/urandom first and /dev/random second. I don't know if posting the code snniplet here is apropiate (licensing reasons). I was also under the impression that /dev/urandom is good enough for most stuff and is generally used for things like this these days (especially because /dev/random blocks). But, I'm not a crypto guy, so I'll defer all that to someone that has better knowledge of the issues involved. -- Bojan
Re: Patch for passing NULL as value argument to apr_env_get
On Tue, 2008-04-29 at 22:41 -0300, César Leonardo Blum Silveira wrote: I'm really sorry if this is annoying, but no one gave any feedback on this. Hasn't anyone liked the idea? Or maybe I should log this on bugzilla? I think this in unlikely to be included, as it changes the usual APR behaviour (which is something you noted yourself). Usually, we just let things segfault if values passed into the function are not correct. BTW, you can solve your problem by wrapping APR functions into your own, where you do perform the checks and/or define needed variables correctly. -- Bojan
Re: Patch for passing NULL as value argument to apr_env_get
On Wed, 2008-04-30 at 22:51 +0200, Branko Čibej wrote: He's proposing to make NULL a well-defined parameter for this function, not an incorrect one. Yeah, that's another way to see it. I was referring to the current behaviour, where NULL is considered an incorrect value (together with a value pointing nowhere), therefore causing a segfault. So, handling NULL specially is avoiding the segfault. -- Bojan
PR #44881
Does anyone mind if I apply the fix mentioned in the bug report: https://issues.apache.org/bugzilla/show_bug.cgi?id=44881 -- Bojan
Re: PR #44881
On Mon, 2008-04-28 at 10:12 -0500, William A. Rowe, Jr. wrote: It's a mailing list, please don't make people chase down a link (remember some of us read our email detached from the web). Citing the patch and bug's subject is a bare minimum :) OOPS, sorry :-( BTW, the same thing was done in Tomcat's Java code, long, long time ago, because /dev/random would block in exactly the same way. -- Bojan
Re: svn commit: r651704 - in /apr/apr/trunk: file_io/unix/copy.c include/arch/netware/apr_arch_file_io.h include/arch/os2/apr_arch_file_io.h include/arch/unix/apr_arch_file_io.h include/arch/win32/apr
On Fri, 2008-04-25 at 15:01 -0700, Roy T. Fielding wrote:
On Apr 25, 2008, at 2:52 PM, [EMAIL PROTECTED] wrote:
==
--- apr/apr/trunk/file_io/unix/copy.c (original)
+++ apr/apr/trunk/file_io/unix/copy.c Fri Apr 25 14:52:36 2008
@@ -54,7 +54,8 @@
/* Copy bytes till the cows come home. */
while (1) {
-char buf[APR_BUFSIZ];
+char buf[BUFSIZ APR_FILE_DEFAULT_BUFSIZE ? BUFSIZ
+ :
APR_FILE_DEFAULT_BUFSIZE];
I'm not sure that is portable C. Is the compiler guaranteed to
optimize that
into a constant?
I would think so. BUFSIZ is defined as constant, as well as
APR_FILE_DEFAULT_SIZE. But, I'll check again...
--
Bojan
Re: svn commit: r651704 - in /apr/apr/trunk: file_io/unix/copy.c include/arch/netware/apr_arch_file_io.h include/arch/os2/apr_arch_file_io.h include/arch/unix/apr_arch_file_io.h include/arch/win32/apr
On Sat, 2008-04-26 at 08:07 +1000, Bojan Smojver wrote: I'm not sure that is portable C. Is the compiler guaranteed to optimize that into a constant? I would think so. BUFSIZ is defined as constant, as well as APR_FILE_DEFAULT_SIZE. But, I'll check again... The book says that such expressions _may_ be evaluated during compilation. So, I guess it is not guaranteed (I would think any modern compiler would do it, though). If you wish, I can have the classic #if (the one I did in header files before) in copy.c. That should make it clear. -- Bojan
Re: svn commit: r651704 - in /apr/apr/trunk: file_io/unix/copy.c include/arch/netware/apr_arch_file_io.h include/arch/os2/apr_arch_file_io.h include/arch/unix/apr_arch_file_io.h include/arch/win32/apr
On Fri, 2008-04-25 at 15:42 -0700, Roy T. Fielding wrote: I just checked by KR 1st edition and it allows constants possibly connected by the ternary operator. So, unless we find a specific compiler that doesn't accept it, I think we can leave it as is. Too late... Already changed :-) -- Bojan
Re: Request for inclusion in 1.2.13/.14 or 1.3.0: Win32 file copy efficiency
On Thu, 2008-04-24 at 13:08 +1000, Bojan Smojver wrote:
Maybe we should have a private #define that takes max of
APR_FILE_DEFAULT_BUFSIZE and BUFSIZ and then uses that instead of just
BUFSIZ?
For instance...
--
Bojan
Index: include/arch/win32/apr_arch_file_io.h
===
--- include/arch/win32/apr_arch_file_io.h (revision 651151)
+++ include/arch/win32/apr_arch_file_io.h (working copy)
@@ -86,6 +86,12 @@
/* For backwards-compat */
#define APR_FILE_BUFSIZE APR_FILE_DEFAULT_BUFSIZE
+#if BUFSIZ APR_FILE_DEFAULT_BUFSIZE
+#define APR_BUFSIZ BUFSIZ
+#else
+#define APR_BUFSIZ APR_FILE_DEFAULT_BUFSIZE
+#endif
+
/* obscure ommissions from msvc's sys/stat.h */
#ifdef _MSC_VER
#define S_IFIFO_S_IFIFO /* pipe */
Index: include/arch/os2/apr_arch_file_io.h
===
--- include/arch/os2/apr_arch_file_io.h (revision 651151)
+++ include/arch/os2/apr_arch_file_io.h (working copy)
@@ -34,6 +34,12 @@
#define APR_FILE_DEFAULT_BUFSIZE 4096
#define APR_FILE_BUFSIZE APR_FILE_DEFAULT_BUFSIZE
+#if BUFSIZ APR_FILE_DEFAULT_BUFSIZE
+#define APR_BUFSIZ BUFSIZ
+#else
+#define APR_BUFSIZ APR_FILE_DEFAULT_BUFSIZE
+#endif
+
struct apr_file_t {
apr_pool_t *pool;
HFILE filedes;
Index: include/arch/unix/apr_arch_file_io.h
===
--- include/arch/unix/apr_arch_file_io.h (revision 651151)
+++ include/arch/unix/apr_arch_file_io.h (working copy)
@@ -90,6 +90,12 @@
/* For backwards-compat */
#define APR_FILE_BUFSIZE APR_FILE_DEFAULT_BUFSIZE
+#if BUFSIZ APR_FILE_DEFAULT_BUFSIZE
+#define APR_BUFSIZ BUFSIZ
+#else
+#define APR_BUFSIZ APR_FILE_DEFAULT_BUFSIZE
+#endif
+
struct apr_file_t {
apr_pool_t *pool;
int filedes;
Index: include/arch/netware/apr_arch_file_io.h
===
--- include/arch/netware/apr_arch_file_io.h (revision 651151)
+++ include/arch/netware/apr_arch_file_io.h (working copy)
@@ -73,6 +73,12 @@
/* For backwards compat */
#define APR_FILE_BUFSIZE APR_FILE_DEFAULT_BUFSIZE
+#if BUFSIZ APR_FILE_DEFAULT_BUFSIZE
+#define APR_BUFSIZ BUFSIZ
+#else
+#define APR_BUFSIZ APR_FILE_DEFAULT_BUFSIZE
+#endif
+
#if APR_HAS_THREADS
#define file_lock(f) do { \
if ((f)-thlock) \
Index: file_io/unix/copy.c
===
--- file_io/unix/copy.c (revision 651151)
+++ file_io/unix/copy.c (working copy)
@@ -54,7 +54,7 @@
/* Copy bytes till the cows come home. */
while (1) {
-char buf[BUFSIZ];
+char buf[APR_BUFSIZ];
apr_size_t bytes_this_time = sizeof(buf);
apr_status_t read_err;
apr_status_t write_err;
Re: Request for inclusion in 1.2.13/.14 or 1.3.0: Win32 file copy efficiency
On Thu, 2008-04-24 at 10:52 +0200, Branko Čibej wrote: The copy buffer size should be a different private value. Feel free to adjust. Reporter of the bug was happy with 4k - that's how the value was chosen, a opposed to 512 bytes he'd get on his platform. -- Bojan
Re: Request for inclusion in 1.2.13/.14 or 1.3.0: Win32 file copy efficiency
On Wed, 2008-04-23 at 21:29 +0200, Erik Huelsmann wrote: Performance would benefit greatly from using APR_FILE_BUFSIZE, which is defined as 4096 bytes. I'm not sure if someone is planning to address this for Windows, but BUFSIZ on glibc systems is 8k and Unix systems seem to have it set to 1k. Maybe we should have a private #define that takes max of APR_FILE_DEFAULT_BUFSIZE and BUFSIZ and then uses that instead of just BUFSIZ? -- Bojan
Re: Rolling APR[-util] 1.3.0
On Fri, 2008-04-11 at 16:41 +0200, William A. Rowe, Jr. wrote: I'd like to proceed with this at the end of the weekend, unless there is significant objection that we can't resolve. +1 and thanks! -- Bojan
Re: apr-util memcache release
On Tue, 2008-04-01 at 10:49 -0700, David Glasser wrote: Is this likely to ever be released in the 0.9 and/or 1.2 branches? Not likely, because versioning rules forbid that: http://apr.apache.org/versioning.html If not, is apr-util 1.3 actively under development? Yes and it will be released eventually as either 1.3 or 2.0. If 2.0, the API you see now in trunk may change. -- Bojan
Re: Possible bug in apr_dbd_mysql
On Thu, 2008-01-31 at 10:36 -0500, Scott Sanders wrote: AuthDBDUserPWQuery to SELECT pwd2 FROM users WHERE email=%s AND active=1. It is Apache's mod_authn_dbd that does the above. This has got to be a bug in the apr_dbd_mysql code Could be. It could also be something to do with mod_authn_dbd. Did you try to run the Apache process through GDB to see what actually makes authentication succeed? -- Bojan
Re: Problems building apr_dbd_mysql.so
On Wed, 2008-01-30 at 11:20 -0500, Scott Sanders wrote: checking mysql.h usability... yes checking mysql.h presence... yes checking for mysql.h... yes checking for mysql_init in -lmysqlclient_r... no ^ checking mysql/mysql.h usability... yes checking mysql/mysql.h presence... yes checking for mysql/mysql.h... yes checking for mysql_init in -lmysqlclient_r... (cached) no ^ Looks like MySQL libraries aren't being found. -- Bojan
Re: [Patch]: Add basic function to APR for LDAP rebind callback support.
Quoting Paul J. Reder [EMAIL PROTECTED]: I addressed the comments and haven't heard back from anyone (Bojan or others). I can't commit to APR and can't commit the Apache portion until the APR part has been committed. I know folks were busy with the latest APR update... Has the dust settled? Any takers? I feel reluctant to commit as I'm not overly familiar with design and implementation of LDAP support in APU. I'll leave that to folks that dealt with LDAP stuff before. Essentially, I just wanted to make sure that your patch comes closer to acceptance, so that LDAP folks are left with less work before committing. -- Bojan
Re: APR 2.0 proposals
On Fri, 2007-11-23 at 10:22 -0600, William A. Rowe, Jr. wrote: are we looking at a 1.3 iteration before we make such radical changes as 2.0? If so, when? I'd love to see 1.3 come together over the next month or two for those who don't want to wait on/depend upon such a major release bump. I think that would be a good idea, given that 1.3 is compatible with 1.2. In terms of when, do we know what in 1.3 is in not-to-be-released-yet status? -- Bojan
Re: database independent connections
On Thu, 2007-11-29 at 16:43 -0500, Bob Rossi wrote: I'm a newb with database connections, so forgive me if this is a simple question. I'm looking into using the dbd routines to open connections to a database. The routines are here, http://apr.apache.org/docs/apr-util/1.2/group___a_p_r___util___d_b_d.html I want to know if this interface allows me to easily swap out the underlying database so I don't get stuck using a particular database. Say perhaps, could the same code use mysql, oracle or db2? Also, how does sqlite play into it? MySQL, yes. Oracle, only with trunk (i.e. unreleased version 1.3 of APU). DB2, no (feel free to contribute the driver! :-). SQLite 2 and 3, yes. Yes, APU DBD is an abstraction layer on top of native database APIs, so you should be able to use the same code with different databases, provided that SQL you pump in is supported by databases in question. I'm looking for high level advice on how to design software that needs to open connections from different sources using the apr API. You can have a look at the source of mod_dbd of Apache, for instance. For a quick a dirty intro, look at the test/testdbd.c program in the tarball of APU. -- Bojan
Re: database independent connections
On Thu, 2007-11-29 at 17:34 -0500, Bob Rossi wrote: OK, perhaps we could have some high level documentation on the webpage for the dbd documentation that describes which databases it works with? http://apr.apache.org/docs/apr-util/1.2/group___a_p_r___util___d_b_d.html#geff12b01f78ac78721acc4a0a318e673 http://apr.apache.org/docs/apr-util/trunk/group___a_p_r___util___d_b_d.html#gbddb1fdcb2f8a5f5b83127485c78e8ae -- Bojan
Re: APR 2.0 proposals
On Mon, 2007-11-26 at 11:55 -0800, Ryan Phillips wrote: What is the benefit in chunking a library? Lower memory footprint, among other things. -- Bojan
Re: svn commit: r598085 - in /apr/site/trunk/docs: download.html index.html
On Sun, 2007-11-25 at 23:11 +, [EMAIL PROTECTED] wrote: liUnix Source: +a href=[preferred]/apr/apr-0.9.17.tar.gzapr-0.9.17.tar.gz/a +a href=[preferred]/apr/apr-0.9.17.tar.bz2apr-0.9.17.tar.bz2/a I can't see these in dist. Have they been copied yet? -- Bojan
Re: [Vote] apr-util-1.2.12 candidate in /dev/dist/
On Wed, 2007-11-21 at 21:32 +0100, Ruediger Pluem wrote: All tests past with the exception of the following error for sqlite2: prepared select Prepare statement failed! (null) Error in prepared select: rc=70023 prepared query Prepare statement failed! (null) Error in prepared query: rc=70023 But this is no regression. SQLite2 does not support prepared statements. -- Bojan
Re: [Vote] apr-util-1.2.12 candidate in /dev/dist/
On Tue, 2007-11-20 at 18:45 -0600, William A. Rowe, Jr. wrote: Please provide your input to release the tarball candidate [-1] APR-util 1.2.12 Fedora 8, i686 and x86_64, testreslist still hangs exactly the same as with 1.2.11. I guess r595990 didn't fix that problem after all. PS. Tested against apr-1.2.12, freshly compiled before apr-util-1.2.12. -- Bojan
Re: [Vote] apr-util-1.2.12 candidate in /dev/dist/
On Thu, 2007-11-22 at 09:28 +1100, Bojan Smojver wrote: I did wait for about 5 minutes before killing it. OK, I'll try again. Ah, yes. I should have waited another minute! Passed test on both architectures, so my vote is now +1. Sorry about the false alarm. -- Bojan
Re: System wide shared library
On Thu, 2007-11-22 at 00:22 +0200, Graham Leggett wrote: Am I correct in understanding this is no longer true (since v1.0)? At least in Fedora/RHEL, it's been a successful system-wide library for a long time now. -- Bojan
Re: [Vote] apr-util-1.2.12 candidate in /dev/dist/
On Thu, 2007-11-22 at 00:25 +0200, Graham Leggett wrote: Did it hang, or did you abort before it was done? I did wait for about 5 minutes before killing it. OK, I'll try again. -- Bojan
Re: cannot commit - no transaction is active
On Mon, 2007-11-19 at 20:38 +0800, Learning apr wrote: I use the function: apr_dbd_transaction_start(driver, pool, sql,transaction); to start a transaction. But when I close this transaction with the function: apr_dbd_transaction_end(driver, pool, transaction); It always gives the error message: cannot commit - no transaction is active What is the reason? Hard to say without knowing what driver you're using. Anyhow, you can always run you code through a debugger and stop to see what actually happens when you attempt to start/end transaction. -- Bojan
Re: [Votes] Apr candidates in /dev/dist/
On Mon, 2007-11-19 at 21:47 +0100, Ruediger Pluem wrote: I haven't commited for testdate so far. Yeah, sorry. I confused a patch sent to the list with a commit. I'll blame it on caffeine deficiency, although old age is more likely ;-) -- Bojan
Re: [Votes] Apr candidates in /dev/dist/
On Thu, 2007-11-15 at 16:26 -0500, William A. Rowe, Jr. wrote: Please provide your input to release. [+1] APR-0.9.17 [+1] APR-1.2.12 [-1] APR-util-1.2.11 [ ] APR-iconv-1.2.1 Fedora 8, i686 and x86_64. Both apr-0.9.17 and apr-1.2.12 fail SHM tests: 0.9.17: - starting consumer. starting producer. Name-based shared memory test FAILED: [2] No such file or directory Name-based shared memory test FAILED: [2] No such file or directory Waiting for producer to exit. Waiting for consumer to exit. Destroying shared memory segment...OK Named shared memory test passed! - 1.2.12: - testshm : FAILED 1 of 6 - However, there have been comments that this is supposed to be expected and that there is no fix available at present. So, I guess +1 in that case. apr-util-1.2.11 fails with: - testdate: FAILED 1 of 2 - Also, testreslist hangs, just like for some other people. Given that Ruediger already committed patches for both of these, it would probably be best to roll again. -- Bojan
Re: [Patch]: Add basic function to APR for LDAP rebind callback support.
Quoting Paul J. Reder [EMAIL PROTECTED]: The patches have been updated to address the issues pointed out below (thanks Bojan). New contents at the old links: http://people.apache.org/~rederpj/APR-trunk_rebind.diff http://people.apache.org/~rederpj/Apache-trunk_rebind.diff Cool. Question: To what end? The xref entry isn't of any use to anyone except in terms of a node in a list that the rebind callback can scan through. The problem is that the rebind callback function is called from the ldap library function and has no context related to the original request info other than the ldap handle passed to it. The xrefs and the list they are kept in needs to be protected to avoid concurrency issues, and having the xref pointer doesn't help the rebind callback function since the xref pointer can't be passed to it. Is there some reason the xref entry would be of use to the caller that I've not thought of? Point taken. I didn't actually look into what the patch does when I reviewed the code. -- Bojan
Re: [Patch]: Add basic function to APR for LDAP rebind callback support.
On Wed, 2007-11-14 at 20:02 -0500, Paul J. Reder wrote: The APR portion of the patch: http://people.apache.org/~rederpj/APR-trunk_rebind.diff Just a few comments (without going into what the patch does): - you probably want to rename apr_ldap_init_xref_lock() to apr_ldap_xref_init() instead - LDAP_rebindproc() should probably be static - what happens when apr_ldap_xref_init() gets called again (e.g. by another Apache module wanting to initialise this subsystem)? - is it possible to make the API so that xref entry is returned instead of being stored for the process? - LDAP_xref_entry_t live is a namespace violation, because it's in a public header file; should probably be called apr_ldap_xref_entry_t -- Bojan
Re: svn commit: r594037 - in /apr/apr-util/trunk: dbd/apr_dbd.c include/private/apr_dbd_internal.h
On Tue, 2007-11-13 at 00:21 -0200, Davi Arnaut wrote: Missing for the #else case. Fixed in r594422. It's the functions supposed to be publicly used? If yes, you should export then properly (apr_dbd.h). If not, it would be better to extend apr_dbd_t with lock and unlock methods which can back point to private functions. They are used by the drivers (which are separate .so/.dll files). I haven't seen any problems on Linux, but apparently on Windows they must be APU_DECLAREd or they cannot be seen. They should not go in apr_dbd.h as they have no purpose outside DBD code. -- Bojan
Re: svn commit: r592216 - in /apr/apr/branches/1.2.x: include/apr_file_io.h test/testdir.c
On Tue, 2007-11-06 at 03:08 +0200, Lucian Adrian Grijincu wrote: What purpose does rv = have in this code? the apr_status_t isn't used anywhere in there A leftover-cut-n-paste purpose? :-) -- Bojan
Re: svn commit: r590501 - in /apr/apr-util/trunk: CHANGES dbd/apr_dbd.c dbd/apr_dbd_freetds.c dbd/apr_dbd_mysql.c dbd/apr_dbd_oracle.c dbd/apr_dbd_pgsql.c dbd/apr_dbd_sqlite2.c dbd/apr_dbd_sqlite3.c i
On Tue, 2007-10-30 at 22:54 +, [EMAIL PROTECTED] wrote: Introduce apr_dbd_open_ex() Given that nobody complained to the patch I sent to the list a while ago, I decided to rely on CTR. Please let me know if any breakage was introduced with this commit. NOTE: FreeTDS driver needs to be fixed to use the new error argument Could someone with knowledge of FreeTDS have a look and implement this new functionality (i.e. return human readable error on failure). At present, this new argument is passed to the driver, but it is unused. -- Bojan
Re: [PATCH] apr_dbd_mysql.c
Quoting Bojan Smojver [EMAIL PROTECTED]: Perhaps something like this (compiles but untested)... And, of course, I've forgotten that we now have FreeTDS driver too. -- Bojan
Re: [PATCH] apr_dbd_mysql.c
On Fri, 2007-10-19 at 15:05 +1000, Bojan Smojver wrote:
Yeah, that's been the problem of the 1.2 API. In 1.3, we cannot do it
with the same function (API/ABI backward compatibility), but we can
introduce another function (e.g. apr_dbd_open_ex()) that does return an
error.
Perhaps something like this (compiles but untested)...
--
Bojan
Index: include/apr_dbd.h
===
--- include/apr_dbd.h (revision 586254)
+++ include/apr_dbd.h (working copy)
@@ -105,6 +105,42 @@
APU_DECLARE(apr_status_t) apr_dbd_get_driver(apr_pool_t *pool, const char *name,
const apr_dbd_driver_t **driver);
+/** apr_dbd_open_ex: open a connection to a backend
+ *
+ * @param pool - working pool
+ * @param params - arguments to driver (implementation-dependent)
+ * @param handle - pointer to handle to return
+ * @param driver - driver struct.
+ * @param error - descriptive error.
+ * @return APR_SUCCESS for success
+ * @return APR_EGENERAL if driver exists but connection failed
+ * @remarks PostgreSQL: the params is passed directly to the PQconnectdb()
+ * function (check PostgreSQL documentation for more details on the syntax).
+ * @remarks SQLite2: the params is split on a colon, with the first part used
+ * as the filename and second part converted to an integer and used as file
+ * mode.
+ * @remarks SQLite3: the params is passed directly to the sqlite3_open()
+ * function as a filename to be opened (check SQLite3 documentation for more
+ * details).
+ * @remarks Oracle: the params can have user, pass, dbname and server
+ * keys, each followed by an equal sign and a value. Such key/value pairs can
+ * be delimited by space, CR, LF, tab, semicolon, vertical bar or comma.
+ * @remarks MySQL: the params can have host, port, user, pass,
+ * dbname, sock, flags fldsz and group keys, each followed by an
+ * equal sign and a value. Such key/value pairs can be delimited by space,
+ * CR, LF, tab, semicolon, vertical bar or comma. For now, flags can only
+ * recognise CLIENT_FOUND_ROWS (check MySQL manual for details). The value
+ * associated with fldsz determines maximum amount of memory (in bytes) for
+ * each of the fields in the result set of prepared statements. By default,
+ * this value is 1 MB. The value associated with group determines which
+ * group from configuration file to use (see MYSQL_READ_DEFAULT_GROUP option
+ * of mysql_options() in MySQL manual).
+ */
+APU_DECLARE(apr_status_t) apr_dbd_open_ex(const apr_dbd_driver_t *driver,
+ apr_pool_t *pool, const char *params,
+ apr_dbd_t **handle,
+ const char **error);
+
/** apr_dbd_open: open a connection to a backend
*
* @param pool - working pool
Index: include/private/apr_dbd_internal.h
===
--- include/private/apr_dbd_internal.h (revision 586254)
+++ include/private/apr_dbd_internal.h (working copy)
@@ -62,10 +62,12 @@
* a lifetime other than a request
*
* @param pool - a pool to use for error messages (if any).
- * @param s - server rec managing the underlying connection/pool.
+ * @param params - connection parameters.
+ * @param error - descriptive error.
* @return database handle, or NULL on error.
*/
-apr_dbd_t *(*open)(apr_pool_t *pool, const char *params);
+apr_dbd_t *(*open)(apr_pool_t *pool, const char *params,
+ const char **error);
/** check_conn: check status of a database connection
*
Index: dbd/apr_dbd_sqlite2.c
===
--- dbd/apr_dbd_sqlite2.c (revision 586254)
+++ dbd/apr_dbd_sqlite2.c (working copy)
@@ -446,8 +446,15 @@
return trans-mode = (mode TXN_MODE_BITS);
}
-static apr_dbd_t *dbd_sqlite_open(apr_pool_t * pool, const char *params_)
+static apr_status_t error_free(void *data)
{
+free(data);
+return APR_SUCCESS;
+}
+
+static apr_dbd_t *dbd_sqlite_open(apr_pool_t * pool, const char *params_,
+ const char **error)
+{
apr_dbd_t *sql;
sqlite *conn = NULL;
char *perm;
@@ -465,8 +472,20 @@
iperms = atoi(perm);
}
-conn = sqlite_open(params, iperms, NULL);
+if (error) {
+*error = NULL;
+conn = sqlite_open(params, iperms, (char **)error);
+
+if (*error) {
+apr_pool_cleanup_register(pool, *error, error_free,
+ apr_pool_cleanup_null);
+}
+}
+else {
+conn = sqlite_open(params, iperms, NULL);
+}
+
sql = apr_pcalloc(pool, sizeof(*sql));
sql-conn = conn;
Index: dbd/apr_dbd_sqlite3.c
===
--- dbd/apr_dbd_sqlite3.c (revision
Re: [PATCH] apr_dbd_mysql.c
On Fri, 2007-10-19 at 14:52 +1000, Ian Holsman wrote: yes thats' debugging, but it would be nice to be able to return the error message when it fails to connect. the 'close' after the real_open removes the error so all you get is the internal error code. Yeah, that's been the problem of the 1.2 API. In 1.3, we cannot do it with the same function (API/ABI backward compatibility), but we can introduce another function (e.g. apr_dbd_open_ex()) that does return an error. interesting, on OS/X it gets one every time, even running it with -X. (when i specify the groups=xyz in the DBDParam setting) Your fix has now been committed to both the trunk and 1.2.x, so hopefully you won't see those segfaults again :-) -- Bojan
Re: [PATCH] apr_dbd_mysql.c
On Fri, 2007-10-19 at 12:31 +1000, Ian Holsman wrote:
{flags, NULL},
+{multi, NULL},
Shouldn't this be part of flags?
+if (fields[8].value != NULL) {
sql-fldsz = atol(fields[7].value);
The above line should also be patched (i.e. this is now index 8, not 7).
if(real_conn == NULL) {
+const char* errstring;
+errstring =mysql_error(sql-conn);
+fprintf(stderr, %s\n,errstring);
I'm guessing the above is your private debugging stuff, right?
@@ -1582,6 +1598,8 @@
static void dbd_mysql_init(apr_pool_t *pool)
{
my_init();
+mysql_thread_init();
Although I can't see any segfaults on Fedora without this, having it in
doesn't upset it either. So, I think we should commit that.
--
Bojan
Re: svn commit: r578076 - /apr/apr-util/trunk/build/dbd.m4
On Fri, 2007-09-21 at 11:55 +, [EMAIL PROTECTED] wrote: Author: jfclere Date: Fri Sep 21 04:55:06 2007 New Revision: 578076 URL: http://svn.apache.org/viewvc?rev=578076view=rev Log: Oops was using the wrong variable. Apparently, this and r578028 is causing people to not be able to link against APU unless libtool is used (e.g. when building PHP, http://news.php.net/php.general/262438). Could you have a look please? -- Bojan
Re: is my apr-util causing downstream bad magic number error?
On Mon, 2007-09-24 at 22:27 -0700, Aliya Harbouri wrote: I'm suspicious of that /usr/local/mysql/lib/mysql/libmysqlclient_r.la; I expected just -lmysqlclient_r. That was a recent addition to the trunk. I'm thinking PHP probably isn't using libtool for linking there and it's trying to link a text file (.la), which obviously isn't going to work. I reckon we should not be exporting this in dbd.m4. Jean-Frederic, could you have a look? -- Bojan
Re: svn commit: r577654 - /apr/apr-util/trunk/build/dbd.m4
On Thu, 2007-09-20 at 09:39 +, [EMAIL PROTECTED] wrote: Author: jfclere Date: Thu Sep 20 02:39:27 2007 New Revision: 577654 URL: http://svn.apache.org/viewvc?rev=577654view=rev Log: Use the mysql libtool library when available. (Otherwise it breaks on one of the HP-UX where I am testing). The second hunk (@@ -158,6 +164,12 @@) doesn't seem correct. The value of $withval at that point is yes, so the code is looking for the presence of yes/lib/mysql/libmysqlclient_r.la or yes/lib/libmysqlclient_r.la. -- Bojan
Re: netinet/sctp.h: present but cannot be compiled
On Mon, 2007-09-17 at 12:29 -0700, Kristis Makris wrote: checking netinet/sctp.h usability... no Can you have a look in config.log to see what actually happened at the above points of configuration? There will be some output about what failed exactly. -- Bojan
Re: netinet/sctp.h: present but cannot be compiled
On Mon, 2007-09-17 at 16:06 -0700, Kristis Makris wrote: I see the following. Perhaps I'm missing some headers ?? There is a macro called APR_FLAG_HEADERS that checks various headers (called from configure.in, defined in build/apr_common.m4), which doesn't seem to be taking into account the fact that one may need to have additional includes for a header to compile. So, the compilation fails. However, there is another macro, APR_CHECK_SCTP (defined in build/apr_network.m4) which is called later in configure.in. Maybe we should be calling AC_CHECK_HEADERS here instead, with correct includes this time. Does anyone have a comment on this? Anyhow, does that second check for SCTP work for you? -- Bojan
Re: netinet/sctp.h: present but cannot be compiled
On Mon, 2007-09-17 at 16:57 -0700, Kristis Makris wrote: I don't know. But APR_CHECK_SCTP is indeed called, so the results should be in the attached config.log. Seems to be OK: --- configure:41928: checking whether SCTP is supported configure:41970: gcc -o conftest -g -O2 -DSOLARIS2=10 -D_POSIX_PTHREAD_SEMANTICS -D_REENTRANT -D_LARGEFILE64_SOURCE conftest.c -luuid -lsendfile -lrt -lsocket -lnsl -lpthread 5 configure:41973: $? = 0 configure:41975: ./conftest configure:41978: $? = 0 configure:41992: result: yes --- So, I think you can safely ignore the first warning. It is caused by a bulk test for existence of headers, which doesn't take into account particulars of compilation requirements. My bet is that APR_CHECK_SCTP exists precisely to remedy that. -- Bojan
Re: Problems with testdate
On Thu, 2007-09-13 at 09:14 +0100, Joe Orton wrote: It looks like the range of the test is supposed to be limited by that of a 32-bit time_t, given the range of dates in year2secs. If you make that array simply an array of time_t or apr_int32_t, and drop the INT64_C() casts, does it work? The problem is in the second test, where this table isn't used. The value of guess comes from the lgc() function, which can return values that are out of range (time_t is signed 32-bit number and lgc() returns unsigned values). Of course, guess, being apr_time_t, happily holds whatever apr_uint32_t can muster and more... -- Bojan
Re: Outdated documentation on http://apr.apache.org/docs/apr/
On Sat, 2007-09-08 at 22:04 +0300, Lucian Adrian Grijincu wrote: Can't this be automated (aka, when a new tag in 1.2 or 0.9 is created to update the docs and when a commit in trunk/include takes place to update the docs? I guess it could be automated. I just logged into my account on people.apache.org to verify if doxygen was present there, but it doesn't seem to be... So, for now, I'll just generate the docs by hand and upload them. -- Bojan
Re: svn commit: r572642 - in /apr/apr-util/trunk: CHANGES INSTALL.MySQL build/dbd.m4 configure.in dbd/apr_dbd_freetds.c dbd/apr_dbd_mysql.c
On Sun, 2007-09-09 at 15:03 -0700, Sander Temme wrote: ./config.nice --with-mysql=/usr Yeah, that's pretty much like the default, as long as /usr/bin is in your path (which on most system it is). ./config.nice --with-mysql=/usr/bin/mysql_config That won't work. You cannot point to the config script, only to location. Let me ask the question differently. What's the location of MySQL libraries, include files and configuration script that you'd like to pick up? -- Bojan
Problems with testdate
Just bumped into this today, but it seems that in function gm_timestr_822() of testdate.c, we are stuffing a 64-bit value (apr_time_t) into something may be a lot shorter (time_t). So, errors start occurring and test fails (Fedora 7, i686). I was thinking that we should patch the test along the lines of the attached. Thoughts? -- Bojan Index: test/testdate.c === --- test/testdate.c (revision 574110) +++ test/testdate.c (working copy) @@ -161,7 +161,14 @@ guess = lgc(guess); if (guess 0) guess *= -1; -secstodate = guess + offset; + +/* make sure it fits into time_t */ +secstodate = (time_t)(guess + offset); + +/* and is positive, taking into account we could be at _MIN */ +if (secstodate 0) +secstodate = -(secstodate + 1); + gm_timestr_822(datestr, secstodate); secstodate *= APR_USEC_PER_SEC; newsecs = apr_date_parse_http(datestr);
Re: svn commit: r574113 - /apr/apr/trunk/test/Makefile.in
On Sun, 2007-09-09 at 21:35 -0700, Sander Temme wrote: Aaaah! What about the other tests later in the sequence? Usually test targets complete every test, regardless of failures. The problem was that nobody ever knew that any of the tests actually failed, because make wouldn't stop. The idea, as understand it, is that if you run make, things should run from beginning to end, unattended. If tests fail midway through the run, then obviously things aren't working and they need attention. Each tester can then get into test directory and run other tests from the sequence by hand, if desired/required. -- Bojan
Re: svn commit: r574113 - /apr/apr/trunk/test/Makefile.in
On Mon, 2007-09-10 at 14:53 +1000, Bojan Smojver wrote: The problem was that nobody ever knew that any of the tests actually failed, because make wouldn't stop. http://issues.apache.org/bugzilla/show_bug.cgi?id=42725 -- Bojan
Re: svn commit: r574113 - /apr/apr/trunk/test/Makefile.in
On Mon, 2007-09-10 at 00:03 -0500, William A. Rowe, Jr. wrote: The full list of failures should be reported, otherwise you only discover the first flaw and have no clue of the scope of the issue. OK, we can do that too, it's not difficult. -- Bojan
Re: svn commit: r574135 - in /apr/apr-util/trunk/test: Makefile.in testdate.c
On Mon, 2007-09-10 at 05:32 +, [EMAIL PROTECTED] wrote: Modified: apr/apr-util/trunk/test/testdate.c Reverted in r574137. This was committed by accident. -- Bojan
Re: svn commit: r574113 - /apr/apr/trunk/test/Makefile.in
On Sun, 2007-09-09 at 22:15 -0700, Sander Temme wrote: Make shouldn't stop, but run all the tests, quietly, and, if there are failures, emit a list of those failures and fail the target. At the end, not at the first failure. Hopefully r574134/5 should be better. Any dramas, let me know. -- Bojan
Re: svn commit: r572642 - in /apr/apr-util/trunk: CHANGES INSTALL.MySQL build/dbd.m4 configure.in dbd/apr_dbd_freetds.c dbd/apr_dbd_mysql.c
On Fri, 2007-09-07 at 12:38 -0700, Sander Temme wrote: If you look at the (long) link line at the bottom, you'll see that it's picking up -L/usr/lib/mysql -lmysqlclient_r , and the root cause of that is probably somewhere in configure where it decides to use the system MySQL despite the fact that we have one of our own. Is that with --with-mysql=/your/own/location/of/mysql? Or without that? -- Bojan
Re: svn commit: r573063 - in /apr/apr-util/branches/1.2.x: CHANGES INSTALL.MySQL build/dbd.m4 dbd/apr_dbd_mysql.c
On Wed, 2007-09-05 at 17:07 -0500, William A. Rowe, Jr. wrote: I'm just pondering, does this violate our ABI rules? Possibly. On the other hand, people that have been plonking the MySQL driver and compiling the result (for which we provided the harness) would have had that problem already. We can avoid the problem (to some extent) by not detecting/compiling MySQL driver by default, but the moment someone turns that feature on (whether in 1.2.11 or in previous versions by adding the driver), they'll face the same problem. -- Bojan
Re: svn commit: r573063 - in /apr/apr-util/branches/1.2.x: CHANGES INSTALL.MySQL build/dbd.m4 dbd/apr_dbd_mysql.c
On Thu, 2007-09-06 at 00:02 +0100, Nick Kew wrote: Aha! Yes, I think that was the plan when we discussed dropping the mysql driver in. Default it to off in the build, and alert users to the fact. Shall do. -- Bojan
Re: New DBD drivers
On Tue, 2007-09-04 at 14:12 +0100, Nick Kew wrote: apr_dbd_mysql is the familiar MySQL driver. Wonderful! Thanks Nick! I'm working on cleaning up the checked in driver (trunk) from all that stuff (i.e. #ifdef's etc.) that is 1.2.x specific. At the same time, if there is support for this, I can backport similarly cleaned (just in the opposite direction) driver to 1.2.x. -- Bojan
Re: New DBD drivers
On Wed, 2007-09-05 at 10:52 +1000, Bojan Smojver wrote: At the same time, if there is support for this, I can backport similarly cleaned (just in the opposite direction) driver to 1.2.x. Ready to commit, just say the word :-) -- Bojan
