boostorg / core

Boost Core Utilities
132 stars 86 forks source link

Fix for sprintf deprecation warning #131

Closed mborland closed 1 year ago

mborland commented 1 year ago

Use same method as https://github.com/boostorg/assert/commit/601ee4ba7ae9d9b532dab5f7e757f4187dce6300.

mborland commented 1 year ago

@pdimov Can you please approve the CI run since I am a first time contributor to this library? Thanks.

pdimov commented 1 year ago

Why is the condition here not the same as the one in Assert?

Lastique commented 1 year ago

I'd prefer a function instead of a macro, something like this. I can make a PR moving that header from Boost.Log to Boost.Core.

And if it is a macro, at least make it private (i.e. BOOST_CORE_DETAIL_SNPRINTF).

mborland commented 1 year ago

Why is the condition here not the same as the one in Assert?

I saw that you have C++98 and 03 in the ci.yaml file, and snprintf is from C++11. If it is superfluous I can remove it.

pdimov commented 1 year ago

I also have C++03 in Assert's CI.

The condition is technically correct, as snprintf is C99 and therefore C++11, but checking __cplusplus like this is true for every MSVC, because they still define it to 199711L.

Since the condition in Assert works, I assume that all supported compilers do provide std::snprintf even in C++98/C++03 mode.

Also, you should #undef the macro afterwards, as it's done in Assert.

mborland commented 1 year ago

Corrections have been made if you could please kick off the CI run again. The undef should allay @Lastique concerns as well.

mborland commented 1 year ago

Pending further review this should be good to merge.