Skip to content

'getenv' considered in MSVC as unsafe function #25

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

AwesomeTornado
Copy link

The function "std::getenv()" is flagged as insecure in Visual Studio (why) , in order to fix this, I changed the functions to "_dupenv_s()" instead.

The code is still the same as before, just with a more secure function.

I tested the changes with one of the example programs, works exactly as before.

@coin-au-carre
Copy link
Member

coin-au-carre commented Aug 17, 2023

Thanks @AwesomeTornado for your work. Your changes do work in Visual Studio but not for other platform. (the CI fails)

I suggest two solutions:

  • We comment that in the README troubleshooting section to warn people using MSVC and having this error. Hence we can keep the current implementation (std::get_env) and we suggest to define the following at compilation:
#define _CRT_SECURE_NO_WARNINGS
  • We ignore the getenv warning with something like:
#ifdef _MSC_VER
#pragma warning(disable: 4996)
#endif

See https://stackoverflow.com/a/66090653/2352158

@coin-au-carre coin-au-carre changed the title changed insecure functions 'getenv' considered in MSVC as unsafe Aug 17, 2023
@coin-au-carre coin-au-carre changed the title 'getenv' considered in MSVC as unsafe 'getenv' considered in MSVC as unsafe function Aug 17, 2023
@coin-au-carre coin-au-carre added the enhancement New feature or request label Aug 17, 2023
@AwesomeTornado
Copy link
Author

Thanks for the quick follow up!

I think that using the pragma to disable the error when compiling with MSVC would be the best option, as it would require the least work from anyone using the library and would not suppress any errors elsewhere in someones code.

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

Successfully merging this pull request may close these issues.

2 participants