Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

get_program_exe_path needs additional porting and improvements #96

Open
johnsonjh opened this issue Oct 23, 2024 · 4 comments
Open

get_program_exe_path needs additional porting and improvements #96

johnsonjh opened this issue Oct 23, 2024 · 4 comments

Comments

@johnsonjh
Copy link
Collaborator

johnsonjh commented Oct 23, 2024

get_program_exe_path() is subject to overflow on long paths and it only works on Linux, Cygwin, and macOS.

I ported the libsir _sir_getappfilename function to work on 17 platforms: Linux, Android, macOS, IBM AIX, IBM i (OS/400), Windows, Cygwin, FreeBSD, NetBSD, OpenBSD, DragonFly BSD, GNU/Hurd, Haiku, Solaris, illumos, SerenityOS, and Emscripten.

This was especially complicated to get working right without too many edge cases on OpenBSD, AIX, and IBM i. This functionality on all platforms is heavily tested and proven.

You can use actually use it as a direct drop-in replacement.

Here is a patch if you want to try it. It assumes libsir in ./libsir:

diff --git a/GNUmakefile b/GNUmakefile
index 2c10fa8..026f81f 100644
--- a/GNUmakefile
+++ b/GNUmakefile
@@ -1,13 +1,13 @@
 CC?=cc
-CFLAGS?=-O3 -flto -Wall -g -Werror=implicit-function-declaration -Werror=int-conversion
-LDLIBS?=-lm
+CFLAGS?=-O3 -flto -Wall -g -Werror=implicit-function-declaration -Werror=int-conversion -I./libsir/include
+LDLIBS?=-L./libsir/build/lib -lsir_s -lm
 INSTALL?=install
 PREFIX?=/usr
 
 OBJS=\
  cpu.o\
  loader.o\
  main.o\
  codepage.o\
  dosnames.o\
  dis.o\
diff --git a/src/utils.c b/src/utils.c
index 138029a..10dcbed 100644
--- a/src/utils.c
+++ b/src/utils.c
@@ -1,38 +1,9 @@
 /* Platform dependent utility functions */
-#include "utils.h"
-#include "dbg.h"
-#include <unistd.h>
 
-#ifdef __APPLE__
-#include <mach-o/dyld.h>
-#endif
+#include "sir.h"
+#include "sir/filesystem.h"
 
 const char *get_program_exe_path(void)
 {
-#if defined(__linux__) || defined(__CYGWIN__)
-
-    static char exe_path[4096] = {
-        0,
-    };
-    if(readlink("/proc/self/exe", exe_path, 4095) == -1)
-        return 0;
-    else
-        return exe_path;
-
-#elif defined(__APPLE__)
-
-    static char exe_path[4096] = {
-        0,
-    };
-    uint32_t length = 4095;
-    if(_NSGetExecutablePath(exe_path, &length))
-        return 0;
-    else
-        return exe_path;
-#else
-
-    /* No implementation */
-    return 0;
-
-#endif
+    return _sir_getappfilename();
 }

I know you probably don't want to bring in libsir as a dependency/subtree/submodule (although it does have a lot of other nice features you could use).

It is actually trivial to get Linux, Android, NetBSD, Solaris, illumos, Dragonfly BSD, Hurd, and SerenityOS working. I've made a PR for that (#97).

Haiku will be easy too.

Extracting the functionality needed to implement this for OpenBSD, AIX, and IBM i (OS/400) — without libsir — would be a much larger project, but is of course possible.

@dmsc
Copy link
Owner

dmsc commented Oct 23, 2024

Hi!

You are right, I do not like depending on external libraries.

In fact, I would like in the future to rewrite the run_emulator function to not execute the emulator, instead it should simply load the executable in the current environment.

This has the advantage of better compatibility (specially for programs that call another program in the same screen - like installers that call unzip, or IDE that call command line compilers).

The disadvantage is worst isolation and the ability for one program to crash another in the emulator - just like old DOS :)

Have Fun!

@johnsonjh
Copy link
Collaborator Author

johnsonjh commented Oct 23, 2024

I'll add FreeBSD and Haiku support later on, it's quite simple.

IBM AIX, OS/400, and OpenBSD support might come later.

@johnsonjh
Copy link
Collaborator Author

FreeBSD and Haiku support added in PR #98

@johnsonjh
Copy link
Collaborator Author

In fact, I would like in the future to rewrite the run_emulator function to not execute the emulator, instead it should simply load the executable in the current environment.

That would be the most portable solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants