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

XLaunchXBE's descendant functions bugs #637

Open
RadWolfie opened this issue Jun 18, 2023 · 5 comments
Open

XLaunchXBE's descendant functions bugs #637

RadWolfie opened this issue Jun 18, 2023 · 5 comments

Comments

@RadWolfie
Copy link
Contributor

I ran a test on:

nxdk/lib/hal/fileio.c

Lines 94 to 99 in 500354e

* d:\foo\bar.txt
* .\foo\bar.txt ==> d:\foo\bar.txt
* \foo\bar.txt ==> d:\foo\bar.txt
* foo\bar.txt ==> d:\foo\bar.txt
* \\.\D:\foo\bar.txt ==> d:\foo\bar.txt
* \??\c:\foo\bar.txt ==> c:\foo\bar.txt

And its output are:

d:\foo\bar.txt test output: \??\D:\foo\bar.txt
.\foo\bar.txt test output: \Device\Harddisk0\Partition1\foo\bar.txt
\foo\bar.txt test output: \Device\Harddisk0\Partition1\foo\bar.txt
foo\bar.txt test output: \Device\Harddisk0\Partition1\foo\bar.txt
\\.\D:\foo\bar.txt test output: \??\D:\foo\bar.txt
\??\d:\foo\bar.txt test output: \??\D:\foo\bar.txt

The more I look at output result... the more it doesn't make any sense to me. Retail console doesn't accept \??\D:\foo\bar.txt path when it should be absolute path. In my opinion, it warrant for rehaul work since nxdk is relying on its own partition table than retrieve full path by using NtOpenSymbolicLinkObject and NtQuerySymbolicLinkObject functions for any mounted drive letter had been done by the title itself. Plus title has the ability to choose which drive letter can be mount on any partitions and child directory.

Seems to be relative to #500 problem. If this whole information should be transfer to #500's post, then I'll transfer it. Below research was done first on a new ticket then realize there are more problems from internal functions.

Below is the original draft issue I made but turns out there are even more issues with nxdk's launch xbe's descendant functions.

If I read XLaunchXBEEx function correctly, it does not look right. From else statement below, it doesn't check for semicolon first incase title wish to set D:\ mount to a parent directory or above by default from reboot process just like retail titles compiled with XDK did. Having two or more semicolons will cause reboot to dashboard or error screen. (For me, it boots to dashboard.)

nxdk/lib/hal/xbox.c

Lines 69 to 86 in 500354e

if (!xbePath) {
launchDataPage->Header.dwLaunchDataType = LDT_LAUNCH_DASHBOARD;
} else {
XConvertDOSFilenameToXBOX(xbePath, launchDataPage->Header.szLaunchPath);
// one last thing... xbePath now looks like:
// \Device\Harddisk0\Partition2\blah\doom.xbe
// but it has to look like:
// \Device\Harddisk0\Partition2\blah;doom.xbe
char *lastSlash = strrchr(launchDataPage->Header.szLaunchPath, '\\');
if (!lastSlash) {
// if we couldn't find a trailing slash, the conversion to
// the xbox path mustn't have worked, so we will return
return;
}
*lastSlash = ';';
}

Beside above issue, I wasn't really sure if it must have semicolon until I ran another test again without any semicolon. The result return as:

LaunchDataPage->Header.szLaunchPath: \Device\Harddisk0\Partition1\Games\reboot\default.xbe
XeImageFileName: \Device\Harddisk0\Partition1\Games\reboot\default.xbe
\??\D: == \Device\Harddisk0\Partition1\Games\reboot\

vs unmodified nxdk:

LaunchDataPage->Header.szLaunchPath: \Device\Harddisk0\Partition1\Games\reboot;default.xbe
XeImageFileName: \Device\Harddisk0\Partition1\Games\reboot\default.xbe
\??\D: == \Device\Harddisk0\Partition1\Games\reboot

The only difference is without semicolon will include backslash whilst with will exclude backslash. But it doesn't affect D drive mounted to read/write contents since I can read the updated log files just fine. If a backslash existed at end of file, in xbox environment it is act as a directory than a "possible" file. Yet the xbox kernel knows to mount it as a directory than a file.


With that said, why semicolon character is a requirement if title doesn't set semicolon in its path to xbe? If I recall, it is entirely optional for title to choose alternative mount point for D drive letter on reboot process.

// if we couldn't find a trailing slash, the conversion to 
// the xbox path mustn't have worked, so we will return

In my opinion, this comment isn't correct. For example, you can have multiple xbes, or re-use the same xbe, from root directory without need to have semicolon character. However, it is also not a requirement as well for child directory wherever xbe file reside in too.

@JayFoxRox
Copy link
Member

it warrant for rehaul work [...]

The OpenXDK code (mostly in hal/ now) is all being replaced by code in nxdk/ (or others, like winapi/ if suitable fit) whenever someone feels like it.

We've tried fixing OpenXDK functions for a while, but there are too many issues with it + the API is often unfixable without breakage.
Hence, using OpenXDK code is more of a temporary measure.

Also see #449

From else statement below, it doesn't check for semicolon first

A proper API design could be an API that supports 2 arguments, one for the folder, and one for the relative XBE path.
The user shouldn't have to know about the semicolon-style path (in my opinion).
I think it could also be worth it, to split initialization of the launchDataPage and actually re-booting.

[...] its own partition table than retrieve full path by using NtOpenSymbolicLinkObject and NtQuerySymbolicLinkObject functions

Yes. Fortunately, by now, XConvertDOSFilenameToXBOX is only used in this function.
There's probably also some winapi function which resolves paths, which could be used in a new implementation.


For now, I'd just recommend to write your own function to launch XBEs as part of you application.
Once you find a good API, create a PR to nxdk.

@RadWolfie
Copy link
Contributor Author

Ah, I missed that #449 ticket. So, recommended method is to rewrite XLaunchXBE relative functions in a new file under more preferably xboxapi folder? Like winapi folder but exclusive for xbox usage.

A proper API design could be an API that supports 2 arguments, one for the folder, and one for the relative XBE path.
The user shouldn't have to know about the semicolon-style path (in my opinion).
I think it could also be worth it, to split initialization of the LaunchDataPage and actually re-booting.

I believe the API for that would be CreateProcessA since it's parameters has lpCurrentDirectory input. Except it may cause some confusion for not getting a return from the function. Perhaps from modern software depending on new process to successfully start before releasing resources. Yes, it is up to title developer to fix their side of the issue to maintenance support for xbox port. Yet likely warrant some form of warning in its API.

BOOL CreateProcessA(
  [in, optional]      LPCSTR                lpApplicationName,   // Can be supported by xbox (relative or path to xbe)
  [in, out, optional] LPSTR                 lpCommandLine,       // May be limited support by xbox (may could be more support within nxdk*?)
  [in, optional]      LPSECURITY_ATTRIBUTES lpProcessAttributes, // Not supported by xbox
  [in, optional]      LPSECURITY_ATTRIBUTES lpThreadAttributes,  // Not supported by xbox
  [in]                BOOL                  bInheritHandles,     // Not supported by xbox
  [in]                DWORD                 dwCreationFlags,     // Some flags may could be supported within nxdk*?
  [in, optional]      LPVOID                lpEnvironment,       // Not supported by xbox (may not be possible within nxdk*?)
  [in, optional]      LPCSTR                lpCurrentDirectory,  // Can be supported by xbox (possible limited to same path to xbe only if xbe isn't made in nxdk*)
  [in]                LPSTARTUPINFOA        lpStartupInfo,       // Some fields may could be supported within nxdk*?
  [out]               LPPROCESS_INFORMATION lpProcessInformation // Not supported by xbox
);

* If launched to same or another xbe that were built by nxdk and may not be compatible with retail titles. Except not sure if this is something we could support in the future.

The other alternative could be WinExec and ShellExecuteA.

@JayFoxRox
Copy link
Member

None of those (CreateProcess / WinExec / ShellExecute) are a good fit.
They are rather strictly specified for Windows and don't make any sense here (as we don't have multi-tasking or a window system).

Instead, I was talking about using a winapi function only for resolving the path (instead of XConvertDOSFilenameToXBOX).. if such a function exists.
For launching an XBE, we'll have to come up with our own function as part of "nxdk/" (nx* functions).
Alternatively, we could also reimplement some XDK function, but I'm not sure if it exists or if it might have other side-effects we can't reproduce (things like D3D interaction.. which we'll need to stub differently for the saved-data / framebuffer persistence).

@RadWolfie
Copy link
Contributor Author

None of those (CreateProcess / WinExec / ShellExecute) are a good fit.
They are rather strictly specified for Windows and don't make any sense here (as we don't have multi-tasking or a window system).

I'm partially in disagreement with you. Xbox system is using modified/stripped version of windows kernel. The reason I suggest those APIs is to able do any debugging perform on Windows platform via make/CMake within the same directory to ensure that changing from one binary to another will work fine.

Instead, I was talking about using a winapi function only for resolving the path (instead of XConvertDOSFilenameToXBOX).. if such a function exists.

Nearest thing I can find is GetFullPathNameA but there are couple possible side effects vs on xbox but that was tested on Windows 10 platform. I'm not sure if Windows 9x or XP era OS does the same thing as on Windows 10.
i.e.

  • \... will translate to top directory of its current drive.
    • This will break \Device\...\... path.
  • \??\... will also do above as well but on hardware I believe is acceptable for not do the conversion to include current drive.

At the time of this writing, there's a lot of inspecting which may will cause performance slowdown. Once integrate with the existing Windows' file APIs of course.

@JayFoxRox
Copy link
Member

The reason I suggest those APIs is to able do any debugging perform on Windows platform via make/CMake within the same directory to ensure that changing from one binary to another will work fine.

That's why you'd implement your own wrapper functions for other platforms (#if NXDK / #if SOME_OTHER_PLATFORM) for debugging purposes (or implement nx* stubs on your development platform).
nxdk is a toolchain and collection of Xbox drivers, not a cross-platform API that guarantees it's APIs are portable - that would defeat the purpose.

Xbox system is using modified/stripped version of windows kernel.

Right, but winapi is the Windows userspace. And the functions that you brought up are dealing with the user-interface or process architecture - the nxdk winapi does not implement multiple processes or the Windows graphics subsystem (same as the XDK / underlying features lacking from Xbox kernel).


We might stub some functionality or partially implement it, but we don't violate assumptions about how winapi works on Windows.

If you use winapi on Xbox, it should either behave as intended, or warn about limitations - but it should never behave differently from winapi elsewhere (e.g. CreateProcess launches a child-process but continues running = a concept that doesn't exist on Xbox = nxdk shouldn't implement it, or only warn about limitation that it can't work).

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

No branches or pull requests

2 participants