Skip to content

Conversation

@rbowler
Copy link

@rbowler rbowler commented Dec 29, 2025

Summary

This change allows declaring files with ORG INDEXED even when no DB backend is present, provided a file-handler is supplied via callfh.

Background

When the compiler has been configured using the --without-db configuration option, then compilation of a program that uses INDEXED files will produce the message runtime is not configured to support ORGANIZATION INDEXED [-Werror=unsupported], and the compilation will fail if the -Werror option is in effect. However, if the compilation option -fcallfh is present, then a database backend is not needed because the indexed file will be handled by the specified external file handler, so the condition should not be flagged as an error or warning.

Motivation

This change enables use-cases where an application or runtime provides the file handler (for example: wrapped file I/O, custom file-systems) and wants indexed access semantics without a DB layer.

Compatibility

This change is backwards compatible for existing programs that rely on DB-backed indexed files; those still work unchanged.

Tests & CI

New regression tests in tests/testsuite.src/run_file.at will:

  • open an ORG INDEXED file using callfh with no DB available and exercise minimal read+write operations with the external file handler writing to and reading from an line-sequential file by changing the type and then using the EXTFH callback.
  • test for the prior failure mode by attempting ORG INDEXED without DB and without callfh. The compile should still fail with an error.

A full run of the test suite / CI will confirm no regressions for DB-backed indexed files.

Copy link
Collaborator

@GitMensch GitMensch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for bringing this up, doing the change and the very good test!
We need a bit of a test tweak, then this is good to go.

Comment on lines 13581 to 13585
# If there is no database and -fcallfh is not specified then
# the compile should still fail with an error
AT_CHECK([$COMPILE prog.cob], [1], [],
[prog.cob:18: error: runtime is not configured to support ORGANIZATION INDEXED; FD TSTFILE
])
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a good test but really belongs into syn_file or syn_misc or even used_binaries - this is also the only part where the "no isam" really applies to; and that test would ideally use the callfh directive (which applies to the following files, so one could be unset, one be set and the last one use EXTFH fopr "normal" file access again)

Can you please move that part out and possible extend as noted?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commit 8f8eca4 moved the compilation test into syn_file and made it conditional on no-isam. I didn't yet include the last test mentioned above (use EXTFH for normal file access) pending clarification of question below regarding strcmp ("EXTFH", f->extfh)

!defined (WITH_DB) && \
!defined (WITH_CISAM) && !defined(WITH_DISAM) && !defined(WITH_VBISAM)
if (f->organization == COB_ORG_INDEXED) {
if (f->organization == COB_ORG_INDEXED && !f->extfh) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this may should check for (!f->extfh || strcmp ("EXTFH", f->extfh)) - a qualified test (see last comment for the test change :-) will verify that

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean that we want to treat -fcallfh=EXTFH the same as if callfh was not specified at all?
If so then I think the test would need to be:
(!f->extfh || (CB_CONST_P(f->extfh) && strcmp ("EXTFH", CB_CONST(f->extfh)->val) == 0))
But what if the user happens to have named the custom file handler as EXTFH? That's a plausible name?

Copy link
Collaborator

@GitMensch GitMensch Dec 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-fcallfh sets the "global" extfh option, thjat should be propagated to f->extfh; also setting the global extfh option (so this may be used to change the next file) is the CALLFH directive.

We therefore only need to check the file's extfh attribute.

Especially to "unset" an external file handler, this can also be set to EXTFH (when you do so in MF you reach the "internal" one) and the GC implementation so far was identical in my view...

The MF/Rocket Software EXTFH name that could be called from C and COBOL is "mFFH", common other ones are "cics_xfh", "cobol_extfh", and ... "EXTFH".

So to answer your last question: If you pass a library or object file to cobc that includes an EXTFH entry point, then yes - that likely would also work (because those are used in the compiler command line before libcob and link order matters).

If you have a COBOL module that does a CALL 'EXTFH' that won't work though, because this is resolved as a system name then (=translated to cob_sys_extfh)...

I'm not sure if/how (and when) to resolve that other than:
a) having a difference between COBOL and C
b) explicit not allow that
c) possibly the best option (other compilers have similar options): add config option similar to reserved but to setup "external" names = name_in_cobol:external_name[:call-convention[:[LDFLAGS][:CFLAGS]]] which we would also read for the elements currently in the system definitions.

Oh: and just to also note that: there's a long standing feature request to make the EXTFH call dynamic (an internal cob_resolve + function call would be done), maybe with -fextfh=+func_name or -fextfh=:func_name.
... and there was also the FR to have the dynamic names be configurable in runtime.cfg (a list to map similar as above, but instead of LDFLAGS the so-name to load for the entry point), which would then automatically handle the -fextfh=:name as well.

@GitMensch
Copy link
Collaborator

Thanks for the good testsuite elements, not sure any more about how to handle -fextfh=EXTFH, I'd leave the tree.c change to your thoughts, "use as currently is" would be ok for me as well "if you use an external file handler, then you should know what you do" is an appropriate way of thinking.
Waiting for these thoughts before doing the upstream commit.

@rbowler
Copy link
Author

rbowler commented Dec 31, 2025

Thanks for the good testsuite elements, not sure any more about how to handle -fextfh=EXTFH, I'd leave the tree.c change to your thoughts, "use as currently is" would be ok for me as well "if you use an external file handler, then you should know what you do" is an appropriate way of thinking. Waiting for these thoughts before doing the upstream commit.

I experimented a bit with the $SET CALLFH directive but I could not get it to produce the results I expected, and I couldn't find any examples that show how it is meant to be used. In view of your previous comments about -fcallfh=EXTFH it seems to me to be an issue best addressed by the feature requests that you mentioned above.

So I would opt for "use the tree.c change as currently is".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants