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

Implement direct.h functions #636

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open

Conversation

ExoSkye
Copy link

@ExoSkye ExoSkye commented Jun 10, 2023

fnmerge and fnsplit haven't been implemented since I noticed inconsistencies in the documentation relating to them, for now I simply have empty function definitions in the form described by https://www.digitalmars.com/rtl/direct.html (ie no _ prefix)

The status of other functions is as follows:
_chdir, _chdrive: impossible to implement, succeeds without an error code, or change of state in the Xbox
_getcwd, _getwd: impossible to implement, return NULL pointers and sets errno to -EINVAL
_getdrive: as before, impossible to implement - returns 0 and sets errno to -EINVAL (this is the error state described by a combo of MS public documentation and digital mars docs)
_searchpath: impossible to implement since there is no PATH variable on the Xbox
_mkdir, _rmdir: implemented, testing required, however

MAXPATH, MAXDRIVE defined, however these may be incorrect right now (they were taken from DOS-era documentation)

Should fix #466 when complete

skye added 2 commits June 10, 2023 14:15
`fnmerge` and `fnsplit` haven't been implemented since I noticed inconsistencies in the documentation relating to them, for now I simply have empty function definitions in the form described by  https://www.digitalmars.com/rtl/direct.html (ie no `_` prefix)
The status of other functions is as follows:
`_chdir`, `_chdrive`: impossible to implement, succeeds without an error code, or change of state in the Xbox
`_getcwd`, `_getwd`: impossible to implement, return NULL pointers and sets `errno` to `-EINVAL`
`_getdrive`: as before, impossible to implement - returns `0` and sets `errno` to `-EINVAL` (this is the error state described by a combo of MS public documentation and digital mars docs)
`_searchpath`: impossible to implement since there is no PATH variable on the Xbox
`_mkdir`, `_rmdir`: implemented, testing required, however
@ExoSkye
Copy link
Author

ExoSkye commented Jun 10, 2023

I've since been informed that getcwd and getwd should be possible to implement by Andy - so I'll implement those when I get home

lib/xboxrt/libc_extensions/direct.c Outdated Show resolved Hide resolved
lib/xboxrt/libc_extensions/direct.h Outdated Show resolved Hide resolved
@ExoSkye
Copy link
Author

ExoSkye commented Jun 16, 2023

I'm going to set this as ready to review since I believe, other than certain stubbed functions, this is ready to go

@ExoSkye ExoSkye marked this pull request as ready for review June 16, 2023 21:57
@ExoSkye
Copy link
Author

ExoSkye commented Jun 16, 2023

I say this, and then completely forget to actually add the .c file to the makefile, whoops, i'll fix that rq

@ExoSkye
Copy link
Author

ExoSkye commented Jun 16, 2023

Can confirm that this built locally with no hiccups

lib/xboxrt/libc_extensions/direct.c Show resolved Hide resolved
lib/xboxrt/libc_extensions/direct.c Show resolved Hide resolved
} else {
DWORD err = GetLastError();

if (err == ERROR_ALREADY_EXISTS) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is ERROR_ALREADY_EXISTS a valid error code for RemoveDirectoryA?

} else {
DWORD err = GetLastError();

if (err == ERROR_ALREADY_EXISTS) {
Copy link
Contributor

@Ryzee119 Ryzee119 Jun 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's alot of cases that are not covered from GetLastError(). ERROR_DISK_FULL or ERROR_FILENAME_EXCED_RANGE is just an obvious example.

Although these will fail, errno will not be set. Not sure if thats a problem. Comment applies to rmdir too.

@ExoSkye
Copy link
Author

ExoSkye commented Jun 18, 2023

Ok, I've implemented a Win32 error code to errno function which just uses a switch statement (I felt that that would be easier for a compiler to optimise, there's even a chance that given there's no IO in it, it could optimise it away completely)

@@ -46,26 +46,28 @@ static int convert_error(DWORD winerror)
*/

int _chdir(char* path) {
return 0;
assert(0);
return -1; // Unreachable
Copy link
Member

@JayFoxRox JayFoxRox Jun 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is reachable when disabling assertions.
In that case, errno must be set, so I suggest setting it to EACCES?

(Other functions, too - but you need to look up appropriate error codes)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah crap, forgot you could do that, I'll sort it out

errno = -EINVAL;
return NULL;
assert(0);
return NULL; // Unreachable
Copy link
Member

@JayFoxRox JayFoxRox Jun 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The macOS man page says:

The function getwd() is a compatibility routine which calls getcwd() with its buf argument and a size of MAXPATHLEN (as defined in the include file <sys/param.h>). Obviously, buf should be at least MAXPATHLEN bytes in length.

Regarding return values:

In addition, getwd() copies the error message associated with errno into the memory referenced by buf.

Similar wording is in POSIX https://pubs.opengroup.org/onlinepubs/009696699/functions/getwd.html

which should be trivial to implement.

I can't find anything about it in the MSDN docs though. Is it contained in the Windows SDK / MS implementations?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, yea, Wikipedia, and digitalmars state that getwd exists so I'm going on that
Should be trivial to implement though

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't find anything about it in the MSDN docs though. Is it contained in the Windows SDK / MS implementations?

I don't see it in https://github.com/microsoft/win32metadata/blob/main/generation/WinSDK/RecompiledIdlHeaders/ucrt/direct.h either.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like getwd is a legacy function which was discontinued. The question is when did it get removed? As for digitalmars state it exists, I don't see getwd mentioned in the referenced link from the OP post. Heck, even did a search for whole site which also has no mention too.

errno = -EINVAL;
return NULL;
assert(0);
return NULL; // Unreachable
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe we can hard-code this for now, so at least apps which use this to find files will work (sometimes some SDK or API will require absolute paths, and then the application might query the base path through this).

I can see how that would be a lot of additional implementation effort, so it can be done in a follow-up PR.


If this follows the same convention, we might be able to use:

nxdk/lib/nxdk/path.c

Lines 9 to 14 in 500354e

void nxGetCurrentXbeNtPath (char *path)
{
// Retrieve the XBE path
strncpy(path, XeImageFileName->Buffer, XeImageFileName->Length);
path[XeImageFileName->Length] = '\0';
}

Otherwise, we might be able to hardcode it to D:\\ which is either (I don't remember) a kernel requirement or at least a dashboard- or self-imposed XBE controlled symlink?

Eitherway, we'd have to follow conventions of the MS implementation (regarding forward vs backward slash, device names vs driver letters etc.).

Similar reasoning for _getdrive, although I speculate that this is required much less.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

D drive letter mount is handled by XDK on boot when D drive letter isn't set by the kernel. Otherwise, it is handled by the kernel on reboot.

Though, I highly recommended to check LaunchDataPage->Header.szLaunchPath first for semicolon character which is mount as working directory by the kernel itself. Then fallback to XeImageFileName with null terminate append at end of string.

fyi: I provided this type of information in XboxDev discord server based on what I had researched years ago.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer XeImageFileName (which the kernel uses to talk to the app / after launching) over LaunchDataPage (which is used by the app to talk to the kernel / before launching)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, LaunchDataPage is used to talk to the kernel but that's only half correct. It is also used by the app as well hence the LaunchData member variable from LAUNCH_DATA_PAGE structure is used to tell the app, either from same xbe or another xbe, what to do next after reboot occur. The kernel doesn't modify LaunchDataPage created by the app. The kernel read from LaunchDataPage->Header structure to do what the app want the kernel to do on reboot.

Here's a log of how it works on first boot and reboot:

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

As you can see XeImageFileName doesn't include semicolon which break working directory mount from D: symlink pointing to a parent directory than the same directory according to reboot process.
This is the same/similar concept on Windows' command prompt or powershell usage. When you change directory from the terminal itself, i.e. C:\logs> . Then execute D:\build\UnitTest.exe which will create log file into C:\logs instead of D:\build directory.

In summary, this is how demo discs and even xbox live works when they focus on D:\ symlink as a working directory. Otherwise attempting to access the contents wouldn't work engineered by original xbox team. I hope this is more clarified for you and everyone else reading this.

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

Successfully merging this pull request may close these issues.

Missing direct.h contents
5 participants