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

winapi improvements #2 #154

Merged
merged 13 commits into from
Sep 25, 2019
Merged

winapi improvements #2 #154

merged 13 commits into from
Sep 25, 2019

Conversation

thrimbor
Copy link
Member

@thrimbor thrimbor commented Aug 24, 2019

Based on #150 (will rebase once that one is merged to solve conflicts).

Most of these commits were done as part of my work to port libc++, but GetTickCount() and GetSystemInfo() were added to reduce nxdk-specific changes to PDCLib's dlmalloc-port (with a few more enhancements to nxdk, it can now build as it does on Win32 without changes).

GetThreadId(), GetCurrentThreadId(), Sleep() and SleepEx were tested separately.

@thrimbor
Copy link
Member Author

Now rebased onto master to fix conflicts.

lib/winapi/profiling.c Outdated Show resolved Hide resolved
lib/winapi/sync.c Outdated Show resolved Hide resolved
@@ -55,13 +55,14 @@ typedef LONG NTSTATUS;
#define STATUS_INVALID_HANDLE ((NTSTATUS)0xC0000008L)
#define STATUS_HANDLE_NOT_CLOSABLE ((NTSTATUS)0xC0000235L)

#define WAIT_FAILED ((NTSTATUS)0xFFFFFFFFL)
#define WAIT_FAILED ((NTSTATUS)0xFFFFFFFFL) // FIXME: Not really an NTSTATUS
Copy link
Member

Choose a reason for hiding this comment

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

What's there to FIXUP here? Consider splitting this off by an empty line and explaining the situation in a comment (if necessary).

Copy link
Member Author

Choose a reason for hiding this comment

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

The fix would be to remove WAIT_FAILED from the kernel headers, but PDCLib currently expects it to be there.

Copy link
Member

Choose a reason for hiding this comment

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

Where should it be? Why can't we move it?

Copy link
Member

Choose a reason for hiding this comment

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

Oh I see - it's in the synchapi stuff, but we can't update PDCLib (to find it), because we'd also pull other PDCLib changes if we update the submodule.

@@ -2111,6 +2080,8 @@ XBAPI VOID NTAPI RtlInitializeCriticalSection
IN PRTL_CRITICAL_SECTION CriticalSection
);

#define RtlDeleteCriticalSection(CriticalSection) ((void)CriticalSection)
Copy link
Member

Choose a reason for hiding this comment

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

This macro still feels dangerous for when CriticalSection is some address math (unlikely, but also shows the problem of macros).
Not sure if an static-function is much better though.

@dracc
Copy link
Contributor

dracc commented Sep 6, 2019

I've looked at the code, found nothing odd that's not already pointed out and those parts have been explained already.
Haven't tried building it (yet), doubt it won't work.

Lgtm. 👍

@JayFoxRox JayFoxRox merged commit ef7735d into XboxDev:master Sep 25, 2019
@JayFoxRox
Copy link
Member

Somehow CI broke for this on merge: https://travis-ci.org/XboxDev/nxdk/jobs/589441844
It still worked during review: https://travis-ci.org/XboxDev/nxdk/jobs/578533447

@thrimbor thrimbor mentioned this pull request Sep 26, 2019
@thrimbor thrimbor deleted the winapi_improved_2 branch September 30, 2019 00:33
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.

3 participants