Microdog40 Standalone Emulator - [merged] #38

Closed voidderef closed 3 years ago

In GitLab by @Cube on Jan 12, 2021, 03:16

Merges microdog40-emu -> master

Summary

Adds a binary called microdog40d, which allows you to run a standalone microdog40 emulator without hooking the binary.

Description

Added a simple wrapper around the hook to run the daemon with a supplied keyfile

How Has This Been Tested?

Tested using this to run Infinity without the dongle

Checklist

[ ] Implemented (unit) test(s) which prove that the introduced changes are working as expected.
Tested with the following games:
- [ Infinity ]
[I think so ] Followed the developer (style) guidelines.
[Not yet] Updated existing doc of or add new doc to README file(s).
[Not yet] Updated development documentation.

In GitLab by @Cube on Jan 12, 2021, 15:36

changed the description

In GitLab by @icex2 on Jan 13, 2021, 20:33

Commented on src/main/sec/microdog40/microdog40.c line 19

I am not sure to what extend this is going to be a conflict, but the commit of this change is already on master. Therefore, you might want to drop it from your MR if that creates a conflict when rebasing and merging.

In GitLab by @icex2 on Jan 13, 2021, 20:38

Commented on Package.mk line 178

Let's create a new category and call it "security":

# Security: Lexical sorting

In GitLab by @icex2 on Jan 13, 2021, 20:38

Commented on Package.mk line 180

$(zipdir)/security.zip: \

In GitLab by @icex2 on Jan 13, 2021, 20:40

Commented on Package.mk line 237

        $(zipdir)/security.zip \
        $(zipdir)/x2hook.zip \

We also need to add the new category zip here that it gets packed into the root distribution zip.

In GitLab by @icex2 on Jan 13, 2021, 20:40

Commented on Package.mk line 257

        $(zipdir)/security.zip \
        $(zipdir)/x2hook.zip \

Same here.

In GitLab by @icex2 on Jan 13, 2021, 20:43

Commented on src/main/sec/microdog40/main.c line 26

Nit: Remove one empty line.

In GitLab by @icex2 on Jan 13, 2021, 20:45

Commented on src/main/sec/microdog40/main.c line 67

I don't see any benefit in running sec_microdog40d_run in a dedicated thread. I would simplify this and just stick the sec_microdog40d_run method in here. The sleep(10) is definitly a good idea to reduce CPU load.

However, it would be nice if the user can also exit the daemon, e.g. by sending a SIGINT to the application (CTRL + C). Do you mind adding that?

In GitLab by @icex2 on Jan 13, 2021, 20:53

Commented on src/main/sec/microdog40/main.c line 9

I would set the log level to debug using util_log_set_level(LOG_LEVEL_DEBUG); to provide verbose output for debugging things.

Optional: If you want to spend the effort, you can add an argument -v to switch between logging with info level and debug.

In GitLab by @icex2 on Jan 13, 2021, 20:54

Commented on src/main/sec/microdog40/main.c line 11

    if (argc != 2) {

Nit, style: No spaces after ( and before )

In GitLab by @icex2 on Jan 13, 2021, 20:58

Commented on src/main/sec/microdog40/main.c line 13

        return 1;

In GitLab by @icex2 on Jan 13, 2021, 21:02

Commented on src/main/sec/microdog40/main.c line 22

There is actually a convenient helper function for loading binary files in util/file.h: util_file_load

if (argc != 2) {
    ...
    return 1;
}

// Note for review (remove when using code): The else is not required as you early return already above. Improves readability

size_t key_data_len;
uint8_t* key_data;

if (util_file_load(argv[1], &key_data, &key_data_len, false) {
    log_error("Loading key file %s failed", argv[1]);
    return 2;
}

// ... do your loop here

util_xfree(&key_data);
// do more cleanup...

In GitLab by @icex2 on Jan 13, 2021, 21:05

Commented on src/main/sec/microdog40/main.c line 33

Actually, I forgot that I even created a daemon module for it...makes sense just re-using that here. Please also re-use sec_microdog40d_is_running in the loop. Call sec_microdog40d_shutdown on SIGINT to terminate the loop.

In GitLab by @Cube on Jan 13, 2021, 21:39

added 1 commit

01db83d3 - Microdog 4.0 Emulator: Standalone

Compare with previous version

In GitLab by @Cube on Jan 13, 2021, 21:41

thanks for the comments, ill start cleaning it up

In GitLab by @Cube on Jan 13, 2021, 22:58

Commented on src/main/sec/microdog40/main.c line 33

I tried adding this, but since sec_microdog40d_is_run_d is checked in a loop that can get blocked waiting on a recvfrom, Testing without actually running a game means, when shutdown is called by the signal, it cannot progress and actually flip sec_microdog40d_is_running to false. Trying to think of a workaround. Probably setting up a timeout with setsockopt is the correct way to handle this.

In GitLab by @icex2 on Jan 13, 2021, 23:45

Commented on src/main/sec/microdog40/main.c line 33

Didn’t consider this, good point. Making the run function non-blocking with a set timeout sounds like a good idea. Can you check and verify that this also works with the current setup in the hooks, e.g. nx2hook?