Cxbx-Reloaded / xbox_kernel_test_suite

Xbox kernel APIs tester written using nxdk
GNU General Public License v3.0
22 stars 6 forks source link

XCreateFile fail to open config.txt #14

Closed Luca1991 closed 4 years ago

Luca1991 commented 6 years ago

For some unknown reason XCreateFile (inside load_conf_file function) is unable to open the config.txt.

I remember this working the last time I checked (first days of Jan). I'll give it a look soon.

Fisherman166 commented 6 years ago

As a temporary fix on the real hardware it is possible to just open "config.txt" and it will look for config.txt in the same directory as the .xbe. Pull Request #15 does this. This solution does not work on the emulator however. This piece of code is the source of the issue because it returns an empty string:

PatrickvL commented 6 years ago

Does that mean strcat doesn't work?

Luca1991 commented 6 years ago

I'm not at home right now, but reading the @Fisherman166 I remembered that getCurrentDirString() is an internal nxdk function (that I copied from fileio.c), and shouldn't be called directly. Anyway I'll fix this today afternoon.

Luca1991 commented 6 years ago

This should be fixed in PR #19

sorry for taking so long....

PatrickvL commented 6 years ago

Does it work now on both real machine and Cxbx-Reloaded?

Luca1991 commented 6 years ago

Sorry I've only tested on my real xbox. I'm on linux, so testing in cxbx might be a bit difficult for me right now.

Fisherman166 commented 6 years ago

My hunch is that it will not work on Cxbx-Reloaded. Once an XBE starts, the filesystem seen by the XBE is the virtual filesystem created by Cxbx, not the host file system. Unless the user has their test suite XBE stored in the internal filesystem, I don't see how the path to look for the config file inside of the test suite would match.

Also, when building with the latest commit, linking fails for me. I also see warnings during compile, which did not exist before.

lld-link: error: /mnt/e/Windows/Programming/nxdk/my_progs/xbox_kernel_test_suite/main.obj: undefined symbol: _strtok_r lld-link: error: /mnt/e/Windows/Programming/nxdk/my_progs/xbox_kernel_test_suite/main.obj: undefined symbol: _strtok lld-link: error: /mnt/e/Windows/Programming/nxdk/my_progs/xbox_kernel_test_suite/main.obj: undefined symbol: _strtol /mnt/e/Windows/Programming/nxdk/my_progs/xbox_kernel_test_suite/../../Makefile:94: recipe for target 'main.exe' failed

Luca1991 commented 6 years ago

@Fisherman166 please update your nxdk to the latest version to fix these linking errors

Fisherman166 commented 6 years ago

I got it compiling. Thanks! As I expected, it does not work on Cxbx-Reloaded due to the difference in host filesystem vs Cxbx filesystem once the XBE has started:

Trying to open config file: config.txt Could not open config file 'config.txt' for read

PatrickvL commented 6 years ago

If a binary would be put up, we could create (and work on) an issue in Cxbx-Reloaded to get it fixed

x1nixmzeng commented 6 years ago

One solution I've found is to use hardcoded paths. To get logging working I had to do this:

open_output_file("c:\\kernel_tests.log");
GXTX commented 4 years ago

nxkd now offers Windows API styled file handling options, CreateFile, WriteFile, ReadFile, etc as merged in XboxDev/nxdk#150 & XboxDev/nxdk#154, I believe these will fix this issue and XCreateFile has been marked depreciated.

PatrickvL commented 4 years ago

There's one issue left, and that's the current dir difference between a real kernel and Cxbx-Reloaded; either it's set to a different path, or it works differently.

In any case, loading config.txt isn't an issue anymore on a real machine. Switching APIs won't change much for that I suspect.

JayFoxRox commented 4 years ago

Changing APIs is highly recommended and urgent. The hal/fileio has been deprecated since late August and will likely be removed sometime this month (possibly the next days).

Users should switch to libc, winapi or kernel level API. If you want to keep using hal/fileio (not recommended) then you'll have to keep a copy of it in your application codebase.

Fisherman166 commented 4 years ago

I went ahead and fixed all of the depreciation issues with the test suite tonight but now I'm running into nxdk compile errors on the newest nxdk commit. One of them for example:

/mnt/e/Windows/Programming/nxdk2/my_progs/xbox_kernel_test_suite/../../lib/winapi/fileio.c:197:13: error: expected expression FILE_NETWORK_OPEN_INFORMATION networkInfo; /mnt/e/Windows/Programming/nxdk2/my_progs/xbox_kernel_test_suite/../../lib/winapi/fileio.c:259:13: error: expected expression FILE_NETWORK_OPEN_INFORMATION networkInfo;

Moving the variable declaration outside of the case statement fixes those errors. Then I get more warnings and then these errors:

/mnt/e/Windows/Programming/nxdk/my_progs/xbox_kernel_test_suite/../../lib/libcxx/include/type_traits:737:50: error: use of undeclared identifier 'char16_t' template <> struct libcpp_is_integral : public true_type {}; ^ /mnt/e/Windows/Programming/nxdk/my_progs/xbox_kernel_test_suite/../../lib/libcxx/include/type_traits:738:50: error: use of undeclared identifier 'char32_t' template <> struct libcpp_is_integral : public true_type {};

Are there bugs in the winapi? This code was committed in July it looks like and I'm wondering why I'm hitting these errors in general files where others should be hitting the same errors.

JayFoxRox commented 4 years ago

Regarding the first error: Hard to tell without having the code (you didn't even say which API you are going to use). Make sure your submodules are up to date and you cleaned your nxdk installation properly. Regarding the second error: libcxx is very new and experimental, but I don't think it should cause any errors on its own. xbox_kernel_test_suite shouldn't depend on it?

I've pinged @thrimbor on Discord about this.

Fisherman166 commented 4 years ago

Using fileapi.h and handelapi.h from lib/winapi. Replaced all XCreateFile with CreateFile, all XCloseFile with CloseHandle, XReadFile with ReadFile, XWriteFile with WriteFile, and XGetFileSize with GetFileSize. I'm assuming its GetFileSize throwing the first errors since it likely uses the seek functions that are throwing the declaration errors.

JayFoxRox commented 4 years ago

Please push your code into a branch and link it here. It's very difficult to discuss this otherwise. I'm unable to reproduce your issue with a diff like this:

diff --git a/main.c b/main.c
index 74d13fc..28847d9 100644
--- a/main.c
+++ b/main.c
@@ -2,7 +2,7 @@
 #include <pbkit/pbkit.h>
 #include <hal/video.h>
 #include <hal/xbox.h>
-#include <hal/fileio.h>
+#include <windows.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -13,26 +13,27 @@
 #include "string_extra.h"

 int load_conf_file(char *config_file_path) {
-    int handle;
-    unsigned int file_size = 0;
+    HANDLE handle;
+    DWORD file_size = 0;
     char *buffer;
-    unsigned int read = 0;
+    DWORD read = 0;
     int result;

     print("Trying to open config file: %s", config_file_path);

-    result = XCreateFile(&handle,
-        config_file_path,
+    handle = CreateFile(config_file_path,
         GENERIC_READ,
         FILE_SHARE_READ,
+        NULL,
         OPEN_EXISTING,
-        FILE_ATTRIBUTE_NORMAL);
-    if(result != 0) {
+        FILE_ATTRIBUTE_NORMAL,
+        NULL);
+    if(handle == NULL) {
         print("Could not open config file '%s' for read", config_file_path);
         return -1;
     }

-    XGetFileSize(handle, &file_size);
+    GetFileSize(handle, &file_size);

     buffer = malloc(file_size);
     if(buffer == NULL) {
@@ -40,13 +41,13 @@ int load_conf_file(char *config_file_path) {
         return -1;
     }

-    result = XReadFile(handle, buffer, file_size, &read);
+    result = ReadFile(handle, buffer, file_size, &read, NULL);
     if(result == 0 || read != file_size) {
         print("Read failed for config file. result = %d, read = %u", file_size, read);
         return -1;
     }

-    XCloseHandle(handle);
+    CloseHandle(handle);

     char *line;
     char *rest = buffer;
@@ -99,7 +100,7 @@ void main(void){
     switch(pb_init()){
         case 0: break;
         default:
-            XSleep(2000);
+            Sleep(2000);
             XReboot();
             return;
     }

(There might be bugs in this patch, I just naively tried to replace functions as you suggested)

The above patch compiles fine. I also made additional search & replace changes which I did not mention here to keep this comment short.

I still assume that your nxdk is not up-to-date / submodules outdated.

Fisherman166 commented 4 years ago

Code is here: https://github.com/Fisherman166/xbox_kernel_test_suite/tree/depreciation_fixes

As I stated in the the first post, I am on the latest commit for nxdk and its submodules. I even cloned a new copy of the repo just to be sure.

JayFoxRox commented 4 years ago

I'm still unable to reproduce that problem on Arch Linux (output of commands omitted):

git clone https://github.com/XboxDev/nxdk.git --recursive --depth=1
cd nxdk && mkdir apps && cd apps
git clone https://github.com/Fisherman166/xbox_kernel_test_suite.git --branch depreciation_fixes
cd xbox_kernel_test_suite
make

Leads to (after a lot of waiting for downloads and compilation):

[...]
[ LD       ] main.exe
[ BUILD    ] /tmp/nxdk/apps/xbox_kernel_test_suite/../../tools/cxbe/cxbe
[ CXBE     ] bin/default.xbe

This is on LLVM 9.0.0 (via clang --version or pacman -Qs llvm).

For MSYS2, also make sure your packages are up-to-date and consistent; MSYS2 can break during incremental updates if you don't do them regularly (at least pacman -Syu every couple of weeks), so it might force a complete re-installation. See MSYS2 docs.

I'd suggest to create an issue on the nxdk repository if this issue persists; however, it does pass CI and we have a handful of active users on different platforms which did not run into any issues. I'm still thinking this is an issue on your end. Please try to repeat the steps outlined above (can probably copy/paste lines to terminal)

Fisherman166 commented 4 years ago

It ended up being a clang issue again (as always). Seems every 6 months my system resets all of the clang symlinks to the system default which is version 3.0. Then I have to go back and recreate the symlinks to point to clang-6.0. Everything is working fine now that I am not pointing to an ancient version of clang.