search

djhackersdev / segatools

Loaders and hardware emulators for SEGA games that run on the Nu and ALLS platforms
The Unlicense
51 stars 6 forks source link

Add API versioning to IO DLLs - [merged] #55

Closed icex2 closed 1 year ago

icex2 commented 3 years ago

In GitLab by @tau on May 23, 2021, 21:31

Merges api -> master

Also includes a bunch of other incidental groundwork, might close and break this up into multiple MRs as this work progresses.

icex2 commented 3 years ago

In GitLab by @tau on May 23, 2021, 21:40

added 2 commits

Compare with previous version

icex2 commented 3 years ago

What's the benefit of linking this staticly?

icex2 commented 3 years ago

That might already be implementation specific information, but some pointers/guidelines regarding what to do in that function might save headaches, e.g. avoid long running operations like blocking reads because it might impact latency etc. (if that's the case). I just remember similar use-cases of the BT5 API.

icex2 commented 3 years ago

Why are you exporting these symbols from the hook dlls? Doesn't the hook sets up all the IO emulation and loads an implementation of the aimeio dll which gets called by the emulation?

icex2 commented 3 years ago

In GitLab by @tau on May 26, 2021, 05:27

Commented on aimeio/meson.build line 1

For support purposes I want to draw a hard line between using the input facilities built into Segatools and any possible third-party extensions of Segatools. I don't want people to get into the habit of overwriting DLLs that ship with Segatools, so instead I'm going to pack the default implementation of the IO DLLs directly into the hook DLLs. If a third-party DLL is used then there is a clear message in the debug log stating that this is the case, the third-party DLL can live in a separate directory (and a path to this third-party DLL is then configured from segatools.ini).

icex2 commented 3 years ago

In GitLab by @tau on May 26, 2021, 05:27

Commented on chunihook/chunihook.def line 5

This is just a matter of internal convenience. If no third-party IO DLL is being used then the hook DLL will call GetProcAddress on itself instead of using LoadLibraryW to load a third-party IO DLL, but it's mostly the same code path in both cases.

The default aimeio can easily be forked and modified by third-party developers.

icex2 commented 3 years ago

approved this merge request

icex2 commented 3 years ago

In GitLab by @Felix on May 27, 2021, 09:57

Commented on aimeio/aimeio.c line 165

Any chance this could be an exact integer size type for developers using programming languages other than C to make an I/O implementation library?

icex2 commented 3 years ago

In GitLab by @Felix on May 27, 2021, 09:57

Commented on aimeio/aimeio.h line 28

If fini is being removed, when should file handles be closed? I thought closing handles in DllMain was frowned upon?

icex2 commented 3 years ago

In GitLab by @Felix on May 31, 2021, 17:08

Commented on aimeio/aimeio.c line 165

Nevermind, most languages that have an FFI have a type that is aliased to the type that C uses.

icex2 commented 3 years ago

In GitLab by @tau on May 31, 2021, 19:04

Commented on aimeio/aimeio.c line 165

changed this line in version 3 of the diff

icex2 commented 3 years ago

In GitLab by @tau on May 31, 2021, 19:04

added 9 commits

Compare with previous version

icex2 commented 3 years ago

In GitLab by @tau on May 31, 2021, 19:06

Commented on aimeio/aimeio.h line 28

The Aime reader cannot be cleanly shut down because the serial port file handle is closed and opened repeatedly much like IIDX EZ-USB. The only difference in the case where aimelib is shutting down is that the file handle does not get opened again.

icex2 commented 3 years ago

Is it save to assume cfg->path != NULL here?

icex2 commented 3 years ago

Wouldn't that be something to rather verify on loading the dll compared to "during runtime" (here)? Thinking about future versions of the dll not exposing all ever function.

icex2 commented 3 years ago

In GitLab by @tau on May 31, 2021, 23:46

Commented on chunihook/chuni-dll.c line 55

cfg->path is an array embedded within the pointed-to struct so it should be safe under the prior assert

icex2 commented 3 years ago

In GitLab by @tau on May 31, 2021, 23:46

Commented on chunihook/jvs.c line 60

That check is already performed by the dll_bind utility. These asserts are an internal check to ensure that the global startup in DllMain has already called the DLL loader, and that its dll_bind_sym table has been declared correctly.

Future versions may have optional exports, the easiest thing to do in that case would be to populate the function pointer struct with built-in stub implementations of each function.

icex2 commented 3 years ago

In GitLab by @tau on May 31, 2021, 23:47

Commented on chunihook/jvs.c line 60

*each missing optional function

icex2 commented 3 years ago

lgtm

icex2 commented 3 years ago

In GitLab by @tau on Jun 12, 2021, 19:02

Commented on aimeio/meson.build line 1

changed this line in version 4 of the diff

icex2 commented 3 years ago

In GitLab by @tau on Jun 12, 2021, 19:02

added 30 commits

Compare with previous version

icex2 commented 3 years ago

In GitLab by @tau on Jun 12, 2021, 19:09

marked this merge request as ready

icex2 commented 3 years ago

Nit, further minor improvement (though it's up to you if you want to take it or leave it): You could provide a macro to provide some support in assembling such a version identifier, e.g. #define API_VERSION(major, minor) ((uint16_t) (((major & 0xFF) << 8) | (minor & 0xFF))

icex2 commented 3 years ago

In GitLab by @tau on Jun 13, 2021, 17:27

added 43 commits

Compare with previous version