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

#include of ASM sources behaves oddly on Linux #31

Closed
snhirsch opened this issue Jun 10, 2023 · 9 comments
Closed

#include of ASM sources behaves oddly on Linux #31

snhirsch opened this issue Jun 10, 2023 · 9 comments

Comments

@snhirsch
Copy link

I'm working with a vintage Z80 project where files in a subdirectory are built by specifying relative paths, e.g.

zasm config/config_fdc.asm -u -w -b cpm22.bin

The source asm file has an #include CPM22.asm pseudo-op. That included file lives in the current working directory (parent of 'config'). Under Linux, zasm is only able to see the included file if its present in the 'config' directory. Based on comments from the project owner, it appears the Windows port of zasm is able to find the file in the current directory. I tried specifying -I . but that did not work. Am I missing something or is this a bug?

@Megatokio
Copy link
Owner

Hi Steven,
"-I" is used by the C compiler, if a C source needs to be compiled first.
Relative paths are resolved starting at the main source file's directory, so i assume you should use
#include "../CPM22.asm".
Please tell me whether this solves the problem or whether the problem lies somewhere else.

@snhirsch
Copy link
Author

I will have to double-check, but my impression is that the Windows build of zasm does not behave in the the same manner as the Linux code.

@snhirsch
Copy link
Author

Definitely a difference in behavior. Here's a test case. Unzip this under both Windows and Linux.

C:\tmp> zasm test\test.asm
assembled file: test\test.asm
    5 lines, 1 pass, 0.0037 sec.
    no errors

hirsch@z87:/tmp$ zasm test/test.asm

in file test.asm:
3: #include "test.inc"
                      ^ file "/tmp/test/test.inc" could not be read: No such file or directory in file "/tmp/test/test.inc" (fd108)
assembled file: test.asm
    3 lines, 1 pass, 0.0022 sec.
    1 error

test.zip

@Megatokio
Copy link
Owner

Megatokio commented Jun 12, 2023

hm, it's the windows version which behaves wrong. as i said, #include "../CPM22.asm" should be correct. the problem is that the windows path separator makes it unmodified into the assembler. when zasm finds a relative path (not starting with "/") it prepends the directory of the main source file to it. the path to the main source file is "test\test.asm" which contains no "/", so nothing is prepended under windows, and "test.inc" is searched in the cwd directly and found there. but this is an error.

i don't own a windows computer so i can not easily test it.

the problem is that the different path separator should be handled by cygwin (if you compile with cygwin) but it isn't. it's probably 10++ years too late to fix this...

a quick search tells me, that most windows api accept "/" as well. So zasm could replace all incoming "\" with "/".
But i would only do this on windows, so specifically if compiled with cygwin. i'll update the source but a new windows binary will take some time.

eventually you can call zasm in a subdirectory using the forward slash. then the paths should resolve just as under unix.

@mxlgv
Copy link
Contributor

mxlgv commented Jul 28, 2023

@Megatokio you can tag me for questions about cygwin, i'll be happy to fix bugs and rebuild.

@mxlgv
Copy link
Contributor

mxlgv commented Aug 14, 2023

@Megatokio. @snhirsch

I have prepared a small patch that fixes this bug. I apologize if the error output is not correct, I do not know how it should be correct, but I know for sure that errno contains an error code when converting the path.

diff --git a/Source/main.cpp b/Source/main.cpp
index 443c8bf..48d0e7d 100644
--- a/Source/main.cpp
+++ b/Source/main.cpp
@@ -46,6 +46,9 @@
 #include <sys/stat.h>
 #include <unistd.h>
 
+#ifdef __CYGWIN__
+	#include <sys/cygwin.h>
+#endif
 
 // static const char appl_name[] = "zasm";
 #define VERSION "4.4.11"
@@ -130,10 +133,17 @@ static cstr help() { return catstr(usingstr(version, compiledatestr()), syntax,
 static cstr posix_path(cstr s) // 4.4.11
 {
 #ifdef __CYGWIN__
-	// fix directory separator under Cygwin (Windows):
-	s = replacedstr(s, '\\', '/');
-#endif
+	static char cyg_path[PATH_MAX];
+	if (cygwin_conv_path (CCP_WIN_A_TO_POSIX | CCP_RELATIVE, s, cyg_path, PATH_MAX))
+	{
+		log("--> convert path %s failed: %s\nzasm: 1 error\n", s, strerror(errno));
+		exit(1);
+	}
+
+	return cyg_path;
+#else
 	return s;
+#endif
 }
 
 static void read_testdir(cstr path, Array<cstr>& filepaths)

I'm also attaching a win64 binary for testing:
zasm-win64.zip

@Megatokio
Copy link
Owner

Hi Turbocat,
sorry i missed your first comment 2 weeks ago, it probably got lost somewhere between the spam mails i get on this account for some time now.
i appreciate your offer to compile versions for Windows!

I pushed version 4.4.12 which includes your fix with a correction: The use of static char cyg_path[] leads to all converted paths using the same buffer.
Can you build and test this version and attach the new Windows binary?
thanks a lot! :-)

@mxlgv
Copy link
Contributor

mxlgv commented Aug 14, 2023

@Megatokio It's OK. I might have noticed this error earlier... Checked and compiled. Everything is working. However, I have a question. Does using dupstr() cause a memory leak?
There is just a trickier way: which can be found in the change history of my comment. But I thought it was redundant.

zasm-win32.zip
zasm-win64.zip

PS:
Fun fact: the path conversion function is so clever that if you use a posix path in the Cygwin terminal, it still works!
I'm not sure about the Win32 version, but it works. Cygwin32 has long been legacy and is not recommended for use.

@Megatokio
Copy link
Owner

Thanks for the binaries, they are already on the project website at k1.spdns.de. :-)

dupstr(): this depends. does malloc() cause a memory leak?
In this case (in main()) it's no memory leak, as the strings are expected to live until the end of the program.
But throughout the assembler (with some exceptions) i intentionally leak the memory for temporary strings, that's to say: i don't bother to free them. zasm typically runs less than 1/10 of a second on a desktop PC and the memory is released when the program exits. If interested, you can take a look at my c-string library at Libraries/cstrings/

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

No branches or pull requests

3 participants