Skip to content
This repository was archived by the owner on Sep 8, 2020. It is now read-only.

osx : fix the inverted version test - #772

Merged
hishamhm merged 1 commit into
hishamhm:masterfrom
pmalhaire:macos_keep_not_high_sierra_ok_fixed
Apr 5, 2018
Merged

hishamhm merged 1 commit into
hishamhm:masterfrom
pmalhaire:macos_keep_not_high_sierra_ok_fixed

Conversation

@pmalhaire

Copy link
Copy Markdown
Contributor

switched flags for this #728

@pmalhaire pmalhaire changed the title fix the switched versions osx : fix the inverted version test Mar 29, 2018
@Explorer09

Explorer09 commented Mar 30, 2018 •

Copy link
Copy Markdown
Contributor

Hello. I wonder what's the reason for inverting the function behavior of CompareKernelVersion() ?
Also, when we are improving the function, is it okay to add static qualifier to CompareKernelVersion? If you are familiar with C you know what it means.

@pmalhaire

pmalhaire commented Mar 30, 2018 •

Copy link
Copy Markdown
Contributor Author

@Explorer09 I do invert the function because I made a mistake when l wrote it. See #728
It makes sense to make it static.

@Smattr Smattr mentioned this pull request Mar 31, 2018
@pmalhaire

pmalhaire commented Mar 31, 2018 •

Copy link
Copy Markdown
Contributor Author

@hishamhm I accidentaly inverted the test when I compacted the code. This is a fix for this

@ylluminarious

Copy link
Copy Markdown

@pmalhaire @Explorer09 After testing this patch on my own and verifying the test with some logging, I can confirm that this patch fixes the problem and it should no longer occur. Unless, of course, something other than DarwinProcess_scanThreads is causing the problem, but there is no evidence to indicate that.

@ylluminarious

Copy link
Copy Markdown

I applied this patch and left the resultant program running for 4 hours on 10.13.3 and didn't crash... I suggest that this patch be merged as soon as possible.

@hishamhm
hishamhm merged commit 0505a7c into hishamhm:master Apr 5, 2018
@hishamhm

hishamhm commented Apr 5, 2018

Copy link
Copy Markdown
Owner

Merged! Thanks, @pmalhaire! A new release is coming this weekend!

@ylluminarious

Copy link
Copy Markdown

@hishamhm Thanks again! Sorry for the false relief in the previous PR... but thanks for your patience!

@pmalhaire

pmalhaire commented Apr 6, 2018 •

Copy link
Copy Markdown
Contributor Author

@hishamhm thanks to you and sorry for the mess I made by inverting my test.
see you at FOSDEM

hishamhm pushed a commit that referenced this pull request Apr 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants