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.

Sign up for GitHub

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

Jump to bottom

Refactor GetGetSystemTimeAsFileTimeFnPtr #84238

Closed
wants to merge 2 commits into from
Closed

Refactor GetGetSystemTimeAsFileTimeFnPtr #84238

wants to merge 2 commits into from

Conversation

xtqqczze
Copy link
Contributor

@xtqqczze xtqqczze commented Apr 3, 2023

  • Don't check whether GetSystemTimePreciseAsFileTime exists, it is always available in win8.
  • Move PInvokes to Interop.Kernel32.
  • Don't use Math.Abs to avoid unecessary overflow check.

@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Apr 3, 2023
@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Apr 3, 2023
@xtqqczze xtqqczze force-pushed the GetGetSystemTimeAsFileTimeFnPtr branch from 3a92325 to a8594fc Compare April 3, 2023 10:43
@xtqqczze
Refactor GetGetSystemTimeAsFileTimeFnPtr
6fbd146 
* Don't check whether `GetSystemTimePreciseAsFileTime` exists, it is always available in `win8`.
* Move PInvokes to `Interop.Kernel32`.
* Don't use `Math.Abs` to avoid unecessary overflow check.
@xtqqczze xtqqczze force-pushed the GetGetSystemTimeAsFileTimeFnPtr branch from a8594fc to 6fbd146 Compare April 3, 2023 11:11
@build-analysis build-analysis bot mentioned this pull request Apr 3, 2023
Closed
@jkotas
Copy link
Member

jkotas commented Apr 3, 2023

Don't check whether GetSystemTimePreciseAsFileTime exists, it is always available in win8

We do not officially support Windows 7. However, we are trying to avoid breaking Windows 7 intentionally for the time being.

@xtqqczze
Copy link
Contributor Author

xtqqczze commented Apr 3, 2023

We do not officially support Windows 7. However, we are trying to avoid breaking Windows 7 intentionally for the time being.

@jkotas It was my understanding this was the case for .NET 7, and that Windows 7 specific code would be removed with .NET 8. Is this still the current thinking?

@stephentoub
Copy link
Member

that Windows 7 specific code would be removed with .NET 8. Is this still the current thinking?

No. We're avoiding removing such code paths.

@xtqqczze xtqqczze force-pushed the GetGetSystemTimeAsFileTimeFnPtr branch from 2aceadf to 42febaa Compare April 3, 2023 14:24
@xtqqczze xtqqczze force-pushed the GetGetSystemTimeAsFileTimeFnPtr branch from 42febaa to fd66b12 Compare April 3, 2023 14:45
@xtqqczze
Copy link
Contributor Author

xtqqczze commented Apr 3, 2023

that Windows 7 specific code would be removed with .NET 8. Is this still the current thinking?

No. We're avoiding removing such code paths.

Closing for now. Has a final decision has been made one way or the other for either .NET 8 or future versions?

@xtqqczze xtqqczze closed this Apr 3, 2023
@ghost ghost locked as resolved and limited conversation to collaborators May 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
community-contribution Indicates that the PR has been added by a community member needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@xtqqczze @jkotas @stephentoub