Use global var for bdb_app_data

[emilienlemaire]

When using DBT->app_data to save the collating sequence function, BDB would not copy it to the DBT passed to bdb_bt_compare after a number of comparison (it seems like this number depends on the size of the data of the record).

The problems originates from BDB as everything is well set before any call to the BDB functions.

To avoid this problem the collating sequence is now always passed to global variable.

[ddeclerck]

Thanks @emilienlemaire .

To add more context, this comes from a client reporting a "bdb_bt_compare was set but no collating sequence was stored in DBT" error (triggered from bdb_bt_compare) when using a key collating sequence on an indexed file.

For some reason, the app_data field of the DBT seems to be reset to NULL at some point when deleting records. This does not occur when using the global variable alternative version intended for older BDB versions that do not have the app_data field. We couldn't figure out if this is a bug in BDB or if we are just missusing this app_data field (comments online suggest it is not intended for public use, cf. this).

I'd suggest we'd just stick with the global variable version. What would you say, @GitMensch ?

[GitMensch]

I'm not sure yet and need some more details.

[GitMensch]

I can reproduce this as well:

libcob: error: bdb_bt_compare was set but no collating sequence was stored in DBT
libcob: warning: implicit CLOSE of READDEL ('READDEL')
BDB0616 Closing already-closed cursor

If you see the last line as well, please check if you can fix that (because that's an unrelated problem, which we normally can't reproduce, but can with the current other implementation).

I'm inspecting the issue that this PR is really about now.

[GitMensch]

OK, this issue really took 2 hours to check. First I've did some debugging within fileio and BDB (using watchpoints), then I've hunted down different version of the docs and headers.

And this is the result:

• BDB 4.1 added it to the header
• BDB 6.0 (docs from 6.0.19 added it to the public API and included the ABI break for the function; note: most GNU/Linux (and BSD) distributions have BDB 5.3

So please

• create a bug report with the findings
• adjust the changelog to have something like "fileio.c: make [DBT_SET_APP_DATA] dependent on DB_VERSION_MAJOR >= 6
• adjust the code as noted

I'll check the the updated version of your PR but am positive that the next one is a "pass".

[GitMensch]

I do wonder - should we place that test

gnucobol/libcob/fileio.c

Lines 929 to 938 in 9afa7a7

	/* LCOV_EXCL_START */
	if (col == NULL) {
	cob_runtime_error ("bdb_bt_compare was set but no collating sequence was stored in DBT");
	cob_hard_failure ();
	}
	if (k1->size != k2->size) {
	cob_runtime_error ("bdb_bt_compare was given keys of different length");
	cob_hard_failure ();
	}
	/* LCOV_EXCL_STOP */

within something like

gnucobol/libcob/move.c

Lines 1035 to 1036 in d5abb19

	#ifndef NDEBUG /* Sanity check to ensure that the data types of both the fields have the
	correct attributes, if not then something is brokend and needs to be fixed */

when changing the related code?

[GitMensch]

good to go in its current state (please mind the updates to integrate it upstream)

[ddeclerck]

Merged on behalf of @emilienlemaire at SVN @ 5398

[GitMensch]

Of course I've missed something...

The line AT_SKIP_IF([test "$COB_HAS_ISAM" != "db"]) should be adjusted - so whoever does a next commit please adjust it (the test will work for any ISAM as the collation is not explicit checked, it is "just used internally").

[ddeclerck]

Noted - maybe when merging #201 .