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

SePlatform.h should not be installed or not redefine system stuff #33

Open
devernay opened this issue Oct 6, 2015 · 3 comments
Open

Comments

@devernay
Copy link

devernay commented Oct 6, 2015

SePlatform.h is included by SeExprBuiltins.h, and installed with the other includes, but it does a few pretty bad things on the windows platform, such as:

#   define snprintf sprintf_s

(which totally breaks snprintf if compiling with mingw for example)

A public library include should not do such things (we had a hard time figuring out what was happening).

There are two alternatives:
1- do not include SePlatform.h in SeExprBuiltins.h, do not install it, and make sure the SeExpr public includes work without it.
2- leave all the #defines out of SePlatform.h and define these in a private include (e.g. SePrivate.h)

devernay added a commit to MrKepzie/openfx-io that referenced this issue Oct 6, 2015
@sopvop
Copy link

sopvop commented Oct 7, 2015

That's how this is fixed in another library akheron/jansson@db0213a

@devernay
Copy link
Author

devernay commented Oct 7, 2015

thanks for the link @sovpop , except I don't like that part:

#  else  /* Other Windows compiller, old definition */
#    define snprintf _snprintf
#    define vsnprintf _vsnprintf
#  endif

because other compilers (e.g. mingw) may provide a proper implementation of snprintf. That's precisely what happens with MinGW.

And in any case, this should not be done in public headers.

@aselle
Copy link
Contributor

aselle commented Oct 16, 2015

We can take a look at fixing this. One other option would be to not use sprintf directly and instead make a function sePrintf() that is defined in a cpp so none of the platform specific stuff gets exported. I.e. we should probably not be defining things to things that might conflict with real functions... So...

#define SeExprSprintf sprintf

kind of defines might be better. I'll admit I haven't tried to build seexpr on windows with mingw32

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

No branches or pull requests

3 participants