WIP: make configuration options machine-readable

[josch]

With the addition of docs/box64.pod, there are now three places that list the box64 configuration options:

• the --help output
• docs/USAGE.md
• docs/box64.pod

To reduce duplication, make maintenance easier and avoid bugs, this pull request is a WIP which tries to encode all options in an array of structs storing function pointers that can handle the option values as well as their documentation in the same place.

In its current state, the --help output is already generated from this array of structs. By extending it, calling box64 with --help=markdown or --help=nroff could easily generate docs/USAGE.md and docs/box64.1, respectively, from the same source.

While implementing this, I already found a few bugs:

• BOX64_DYNAREC_FORCED is not documented
• BOX64_FIX_64BIT_INODES is not documented
• the box64_malloc_hack==1 conditional is never executed because it's inside the body of a box64_malloc_hack if clause

What do you think of this approach? If you think it's a good idea, then i'll extend it to cover the remaining environment variables and output markdown as well as nroff directly.

[josch]

Hi, could you give me some quick feedback on the idea behind this pull request? Otherwise I fear too much time passes and I'd have to re-learn box64 internals. :)

[ptitSeb]

It looks good.
I just think it should also impact rcfile stuffs.

[ptitSeb]

I think BOX64_FIX_64BIT_INODES should be removed, it's a copy/paste from box86 and is probably not relevant in box64 case

[ptitSeb]

I need to re-check BOX64_DYNAREC_FORCED but not documenting it was on purpose IIRC.

[ptitSeb]

box64_malloc_hack is still super hacky, I'm not sure it will end up as it is now.

[josch]

Thank you for your comments!

I just think it should also impact rcfile stuffs.

Yes, I purposefully didn't work on that to not sink time into a project that you don't like. Would you mind switching out the current ini parser for one like https://github.com/benhoyt/inih which is available in Debian and used by lots of other projects? It wouldn't require an additional build dependency, because the library is tiny and can just be copy-pasted into the source tree.

I think BOX64_FIX_64BIT_INODES should be removed

Okay, I'll remove it.

I need to re-check BOX64_DYNAREC_FORCED but not documenting it was on purpose IIRC.

Okay, then my struct needs another flag for configuration options that should not produce documentation.

box64_malloc_hack is still super hacky, I'm not sure it will end up as it is now.

You can still remove it independent of this pull request. I was more wondering what you think about the general idea of consolidating all options and their parsing into a unified structure that at the same time produces documentation.

[ptitSeb]

I'm not a big fan of external dependancies for box. So I would prefer to stick to "in-house" solution if possible.

The unified structure for all documentation is great, but the interaction with rcfile should be thought in the conception phase, not as a second thought, because the interactions with the various option and when/how to apply them is not always trivial.

[josch]

Yes, that's why I want to add an update to the rcfile reading at the same time in this pull request. That's also why I stumbled upon what made me file #652 😄

Yes, I noticed that there is a lot going on in terms of which configuration options trigger what in which order. But currently these operations are mixed with the retrieving of configuration options itself. My goal is to separate the two and have one place that retrieves the configuration from the environment and from the rcfile and one place that then implements the resulting logic to have a clear separation between these two parts of the code, hopefully making everything easier.

I'm sure that my next revision of this pull request will introduce bugs because some things are not happening anymore in the order they should but I hope the process of weeding these out is worth the end result of having everything nicely compartmentalized and hopefully more maintainable in the future.

To this end I also wondered why there is a distinction between some variables that are global and then used as extern from other parts of the code and some variables that are stored in box64context_t *my_context which (despite what its name suggests) is also a global instead of being passed around as a function argument. Why not have everything inside the "context" or have everything as individual globals?

[ptitSeb]

box64context_t was initialy a proper context. But after a while I realized that I don't need that context to be local, it's really is a global stuff, and the actual "context" is in fact x64emu_t
But it took me a lot of iteration to end up with this organisation, and I have not really done a huge cleanup of that.
I keep the box64_XXXX variable outside of context because I consider them more globals then the global context (if that make sense)