First working C Implementation

[DarkShadow44]

Tests: kaitai-io/kaitai_struct_tests#97
Compiler: kaitai-io/kaitai_struct_compiler#253

[SamuelMarks]

@DarkShadow44 Great to see!

Also it might a good idea to export only the symbols that are intended for shared libraries. Useful for reducing symbol table size, FFI, and for when visibility flags are enabled in you compiler frontend invocation.

For more details see:
https://gcc.gnu.org/wiki/Visibility

If you don't want to generate your 'export header' automatically (e.g., in CMake with GenerateExportHeader) then you can use something like my:
https://github.com/SamuelMarks/cmocka/blob/msvc2005/pregen/cmocka_export.h

(then just use KAITAI_EXPORT before every intended-exported symbol)

Happy to contribute the patch just say the word

[DarkShadow44]

Well, the entire runtime isn't designed to be used as a shared library in the first place. It's meant to be simply included as a single c file into the project. That's also why the header has special logic to handle dependencies and "configuring" the runtime. Not sure if the visibility really matters in such circumstances?

Btw, I'm not 100% happy with how I currently handle KS_DEPEND_ON_INTERNALS, so I might rework that to have a proper interface, then only the runtime kaitaistruct.c would have access to internal structs and such.

[SamuelMarks]

@DarkShadow44 Interesting.

My interest is on the FFI side. Namely I'm thinking that once this is ported, then the number of supported languages grows from not 11 to 12, but from 11 to 50+.

(I'm working on a new C compiler and associated tooling to take C libraries + headers and generate packages in dozens of different languages—using language-ecosystem recommended tooling—and release to their various app stores [e.g., Rust cargo crates.io; Python pip pypi.org] in an easily pipelinable [e.g., for CI/CD] fashion)

I don't want to translate things out of this C header that don't make sense when other languages are calling into it.

[DarkShadow44]

Well, I separated the functions into functions that are needed by the program and functions needed by the generated code only. I could potentially separate this into two headers, if that makes sense?
Although you'd need to write a little stub program to handle ks_config_create since that's a header only function (because of the mentioned dependencies handling). But since the generated files are part of the FFI anyways, you need to have your own program with the generated .c files, so I don't think that's too big of an issue.
Anyways, if you have suggestions for improvements, feel free to tell me.

[soer7022]

What is the timeline for this being merged and released so it can be used?

[jhgorse]

@GreyCat @generalmimon what would you like to see before accepting the initial C implementation? Cheers.

[generalmimon]

@jhgorse:

what would you like to see before accepting the initial C implementation?

I would like to see you (or someone who has the time, which is not me at the moment) reviewing and testing the code. Not only in this PR, but also in related PRs. Check out kaitai-io/kaitai_struct_compiler#253 locally, rebase everything on top of master branches, build the compiler from source (as in https://doc.kaitai.io/serialization.html#_building_the_compiler_from_source), run the tests in kaitai-io/kaitai_struct_tests#97 (hint: use build-formats and spec_kst_to_all), see which fail to generate and which fail at runtime and why. Run linters and static code analysis tools on both the runtime library and the generated code. Run the tests and the runtime library under sanitizers, for example https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html. Read the code in all 3 PRs, report code smells and question why things are done this way and not another way. Pick a format in https://github.com/kaitai-io/kaitai_struct_formats, generate a C parser for it and try to extract some useful data from the format - how does it feel to use the generated parser?

[generalmimon]

Right off the bat, the indentation is wrong, for example here:

kaitai_struct_c_runtime/kaitaistruct.c

Lines 770 to 772 in e7a7971

	case KS_TYPE_ARRAY_UINT:
	switch(handle->type_size)
	{

kaitai_struct_c_runtime/kaitaistruct.c

Lines 781 to 784 in e7a7971

	}
	case KS_TYPE_ARRAY_INT:
	switch(handle->type_size)
	{

... or here (3-space indent? 🤨):

kaitai_struct_c_runtime/kaitaistruct.c

Lines 800 to 804 in e7a7971

	static double array_get_float(ks_usertype_generic* array, void* data)
	{
	ks_handle* handle = array->handle;
	if (handle->type == KS_TYPE_ARRAY_FLOAT)
	{

Also, this is a terrible idea:

kaitai_struct_tests / translator/src/main/scala/io/kaitai/struct/testtranslator/specgenerators/CSG.scala:40-43

  override def runParseExpectError(exception: KSError): Unit = {
    runParseCommon1()
    out.puts(s"BOOST_CHECK_EQUAL(error == 0, 0);")
  }

I explained why in kaitai-io/kaitai_struct_tests#96 (review):

Testing the error type is actually very important - if you don't do that, you're just blindly accepting any error that may arise for any reason (which is very prone to producing false negative test results, because the test might as well just crash with some exception before it even starts, but the test result will be "all good").

I've seen this before in Lua - (...)

---

Anyone could probably find this in 5 minutes if someone actually looked at the code instead of expecting someone else to do it.