Modules and in-structure ifdefs don't work together

[kitor]

Found with file_man.mo: Digic 7 and 8 have a different fio_file struct, with additional fields. Modules are universal, compiled without per-cam ifdefs and thus wrong structure definition is used, ending with file manager wrong behavior.

Hardcoding struct to Digic78 variant made it usable on Digic78 models, but obviously broken for older ones.

This apply to any structure or function interface ifdef that may exists in code. We usually wrap functions into old interfaces, but that's impossible for structures.

[reticulatedpines]

As discussed, long term this should probably be a significant change to module system, so they can only use blessed ML APIs. Attempts to access other structs, apis etc should fail at compile time. This may mean they can only link against ml_api.o, something like that, and we have to expose things from e.g. stubs.o indirectly. Build system would restrict include paths for headers and ml_api.h would be in a separate dir. Maybe that include path restriction is enough on its own.

This may not be easy work. See adtg_gui.c, which does a lot of this: 1016 else if (is_camera("5D2", "2.1.2")) 1017 { 1018 ADTG_WRITE_FUNC = 0xffa35cbc; 1019 CMOS_WRITE_FUNC = 0xffa35e70; 1020 }

That would be better as an ML api that returned the func addresses.

There's some existing way to set module api version, we should bump that.

[kitor]

This may be fun, considering LUA module tries to expose every possible functionality.

[reticulatedpines]

Attempt at a plan-to-fix:

• audit each module, creating an issue if required, listing all violations of cam-independent behaviour (one issue per module)
• using probably file_man.mo, do proof-of-concept changes so file_man.mo doesn't depend on ML source in any way. This probably means something like:
  • remove all refs to ML source, e.g. includes, use of any symbols not exported by magiclantern.o, etc
  • create some helper functions in ML to provide info file_man currently gets directly (e.g., offsets into structures)
• convert other modules in a similar way
• modify module build process to remove all access to ML include dirs, etc. If modules are to be independent from ML builds, this must be enforced at module build time as much as possible. The existing dep checker which copies modules into zip already checks if module requirements are met by exports from ML, I am hopeful we can keep this as the only mechanism (all modules will always build, each cam will only include the module if deps are satisfied, and if an unsatisfied module is included manually it will cleanly fail to load). Sadly we can't stop people casting constants to pointers, etc, which is currently done, this kind of thing we just have to audit for.
• test that attempts by module source to use e.g. CONFIG_DIGIC or include dryos.h fail to build

module.h contains MODULE_MAJOR. The work for this ticket will bump it to 9.

[reticulatedpines]

module.h is included both by modules, and some files in src, e.g. lens.c. Turns out ML can call into module code, and if the symbol doesn't exist (when module isn't loaded) it's supposed to return 0. Feels messy.

Maybe we can split this file? Maybe we move it to e.g. include/modules/module.h which only contains that file, so modules can cleanly take it but not anything else?