After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 412122 - mdb_sql_bind_column API
mdb_sql_bind_column API
Status: RESOLVED FIXED
Product: libgda
Classification: Other
Component: MDB provider
2.99.x
Other All
: Normal critical
: ---
Assigned To: gnome-db Maintainers
gnome-db Maintainers
: 162471 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2007-02-26 03:13 UTC by Yaakov Selkowitz
Modified: 2007-03-19 18:58 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
libgda-mdb-fix.patch (4.68 KB, patch)
2007-03-14 04:14 UTC, Yaakov Selkowitz
none Details | Review
config.log (62.25 KB, text/plain)
2007-03-16 18:01 UTC, Murray Cumming
  Details
config.log (63.64 KB, text/plain)
2007-03-16 18:08 UTC, Murray Cumming
  Details

Description Yaakov Selkowitz 2007-02-26 03:13:41 UTC
This change:

2007-02-08  Murray Cumming  <murrayc...>
* providers/mdb/gda-mdb-provider.c: (gda_mdb_provider_execute_sql): Provide an int* output parameter to mdb_sql_bind_column(), though we do not do anything with it yet. This fixes the build, at least with the mdbtools 0.5.99.0.6 version in my Ubuntu Edgy, which I guess is fairly recent.

breaks the build with the upstream 0.6pre1 tarball (released 2004-06-18).  The fourth argument to mdb_sql_bind_column was added in CVS the following December.

http://mdbtools.cvs.sourceforge.net/mdbtools/mdbtools/src/sql/mdbsql.c

I'm not sure how best to deal with this.
Comment 1 Murray Cumming 2007-02-28 17:17:21 UTC
2004-06-18 seems old. What's the latest tarball version? In general, I would rather target the latest tarball than an unreleased version.

If possible, could you provide a patch that #ifdefs around this part of the code?
Comment 2 Yaakov Selkowitz 2007-02-28 17:58:06 UTC
> 2004-06-18 seems old. What's the latest tarball version?

0.6pre1

> In general, I would rather target the latest tarball than an unreleased version.

Agreed, but:

* Debian and Ubuntu are packaging a CVS build ("0.5.99.x", 20050409 or 20051109)
* most .rpm's I found via Google were either 0.5 or 0.6pre1
* Gentoo has 0.5 and 0.6pre1, but no CVS build.
* SUSE 10.1 has 0.5.

> If possible, could you provide a patch that #ifdefs around this part of the code?

The main difference between 0.5 and 0.6pre1 was write support.  The latter defined MDB_WRITABLE and took three args to mdb_open(); in 0.5 it took only 2.  So #ifdef MDB_WRITABLE should take care of that.  I don't remember if there was anything else.

These CVS builds are trickier, because they add this extra arg to mdb_sql_bind_column; this might require a configure test.

Comment 3 Yaakov Selkowitz 2007-02-28 18:04:01 UTC
Sorry, make that two args to mdb_open() vs one.
Comment 4 Yaakov Selkowitz 2007-02-28 19:32:28 UTC
This is hand made and may not apply, but I won't be able to generate a patch until tonight at least.

--- configure.in
+++ configure.in
@ -806,6 +806,9 @
 		MDB_CFLAGS=-I${mdbdir}/include
 		MDB_LIBS="-L${mdbdir}/lib -lmdb -lmdbsql"
 		AC_DEFINE(HAVE_MDB, 1, [Have MDB])
+		AC_CHECK_LIB(mdbsql, mdb_sql_bind_len, ,
+			AC_DEFINE(MDB_SQL_BIND_COLUMN_TAKES_LEN, 1, [Have MDB  with four-argument mdb_sql_bind_column]),
+			'-lmdb')
 	fi
 fi
 
--- providers/mdb/gda-mdb-provider.c
+++ providers/mdb/gda-mdb-provider.c
@ -209,7 +209,11 @
 	mdb_cnc = g_new0 (GdaMdbConnection, 1);
 	mdb_cnc->cnc = cnc;
 	mdb_cnc->server_version = NULL;
+#ifdef MDB_WRITABLE
 	mdb_cnc->mdb = mdb_open (filename, MDB_WRITABLE);
+#else
+	mdb_cnc->mdb = mdb_open (filename);
+#endif
 	if (!mdb_cnc->mdb) {
 		gda_connection_add_event_string (cnc, _("Could not open file %s"), filename);
 		g_free (mdb_cnc);
@@ -794,11 +798,12 @@
 		bound_data[c] = (gchar *) malloc (MDB_BIND_SIZE);
 		bound_data[c][0] = '\0';
 
-		/* Note that older versions of MDB don't have the 4th len_ptr parameter.
-		 * Please submit a patch with a suitable #ifdef if you need this to build with the older version.
-		 */
+#ifdef MDB_SQL_BIND_COLUMN_TAKES_LEN
                 int len = 0; /* Maybe we should actually do something with this output parameter. */
 		mdb_sql_bind_column (mdb_SQL, c + 1, bound_data[c], &len);
+#else
+		mdb_sql_bind_column (mdb_SQL, c + 1, bound_data[c]);
+#endif
 		
 		/* set description for the field */
 		fa = gda_data_model_describe_column (model, c);
Comment 5 Yaakov Selkowitz 2007-03-13 13:46:48 UTC
Oops, MDB_WRITABLE is an enum, not a CPP macro, which makes this a little harder.  A real patch for libgda trunk should be ready tonight.
Comment 6 Yaakov Selkowitz 2007-03-14 04:14:37 UTC
Created attachment 84551 [details] [review]
libgda-mdb-fix.patch

* configure.in:
* providers/mdb/gda-mdb-provider.c:
Use autoconf tests to handle API changes in mdbtools.  Fixes bug 412122.
Comment 7 Murray Cumming 2007-03-15 20:58:52 UTC
Thanks for working on this.

It breaks the build for me, on Ubuntu Feisty with 0.5.99.0.6pre1.0.2005110903 like so:

checking for MDB Tools files... found MDB Tools in /usr
checking whether mdb_open takes one or two arguments... two
checking whether mdb_sql_bind_column takes three or four arguments... three
...
gda-mdb-provider.c: In function 'gda_mdb_provider_execute_sql':
gda-mdb-provider.c:805: error: too few arguments to function 'mdb_sql_bind_column'

I think this is because the C macro is not actually defined anywhere. There is an acconfig.h file, in which it could be undefed, but that file does not seem to be used in the build system.

Comment 8 Yaakov Selkowitz 2007-03-15 21:34:20 UTC
What error is shown in config.log for the mdb_sql_bind_column test?
Comment 9 Yaakov Selkowitz 2007-03-15 22:09:18 UTC
Just looked this over again.  Are you using '-Wall -Werror'?  If you got something like the following:

conftest.c: In function `main':
conftest.c:10: warning: 'c' might be used uninitialized in this function

then change the 'int c;' declaration to 'int c=0;'.

Other than that, I would need to see the relevant error in config.log.
Comment 10 Murray Cumming 2007-03-16 18:01:21 UTC
Created attachment 84731 [details]
config.log
Comment 11 Murray Cumming 2007-03-16 18:08:33 UTC
Created attachment 84733 [details]
config.log

Sorry, ignore the previous one.

So, the problem seems to be the [ character is removed.
Comment 12 Murray Cumming 2007-03-16 18:12:29 UTC
If I double quote with [[ then the build works, so I'll commit this. Thanks.
Comment 13 Yaakov Selkowitz 2007-03-16 18:34:48 UTC
Oh, of course, m4 quoting.  Would \[ and \]work as well?
Comment 14 Murray Cumming 2007-03-19 18:58:46 UTC
*** Bug 162471 has been marked as a duplicate of this bug. ***