-
Notifications
You must be signed in to change notification settings - Fork 69
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
base: master
Are you sure you want to change the base?
Conversation
`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
I've since been informed that |
I'm going to set this as ready to review since I believe, other than certain stubbed functions, this is ready to go |
I say this, and then completely forget to actually add the |
Can confirm that this built locally with no hiccups |
lib/xboxrt/libc_extensions/direct.c
Outdated
} else { | ||
DWORD err = GetLastError(); | ||
|
||
if (err == ERROR_ALREADY_EXISTS) { |
There was a problem hiding this comment.
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
?
lib/xboxrt/libc_extensions/direct.c
Outdated
} else { | ||
DWORD err = GetLastError(); | ||
|
||
if (err == ERROR_ALREADY_EXISTS) { |
There was a problem hiding this comment.
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.
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 |
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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:
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
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.
fnmerge
andfnsplit
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 setserrno
to-EINVAL
_getdrive
: as before, impossible to implement - returns0
and setserrno
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, howeverMAXPATH
,MAXDRIVE
defined, however these may be incorrect right now (they were taken from DOS-era documentation)Should fix #466 when complete