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

use 'const' and 'static' where it is required #1606

Closed
wants to merge 0 commits into from

Conversation

dpronin
Copy link
Contributor

@dpronin dpronin commented Jul 28, 2023

protect variables from programmer's point of view and hide several by 'static' variables

Signed-off-by: Denis Pronin dannftk@yandex.ru

@dpronin
Copy link
Contributor Author

dpronin commented Jul 28, 2023

@axboe are there any ideas why constness and/or static of some variables/constants and/or parameters may affect runtime tests? I don't have one

@vincentkfu
Copy link
Collaborator

I don't know why but your changes make the test jobs segfault on windows.

@dpronin
Copy link
Contributor Author

dpronin commented Jul 28, 2023

I presume, because I used to be a developer under windows, static const variables Windows compiler places in read-only segment section in binary that is mapped within PAGE with READ ONLY protection, and probably, there is some code that attempts to write to this section/mapped page

@vincentkfu
Copy link
Collaborator

Actually the Windows failures started with the double free fix. So this patch is not the cause.

@dpronin
Copy link
Contributor Author

dpronin commented Jul 28, 2023

Actually the Windows failures started with the double free fix. So this patch is not the cause.

Is there any opportunity to check these tests without launching up Windows as a virtual machine? Is there an emulator of cygwin under Linux?

@vincentkfu
Copy link
Collaborator

None that I know of.

@dpronin
Copy link
Contributor Author

dpronin commented Jul 28, 2023

It's a pity

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

Successfully merging this pull request may close these issues.

2 participants