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

Opals tuw no std string symbols #321

Closed
wants to merge 7 commits into from
Closed

Opals tuw no std string symbols #321

wants to merge 7 commits into from

Conversation

opalsTUW
Copy link
Contributor

resolution for defect #7254

@@ -338,7 +338,11 @@ typedef unsigned int GUIntptr_t;

#ifndef CPL_DLL
#if defined(_MSC_VER) && !defined(CPL_DISABLE_DLL)
# define CPL_DLL __declspec(dllexport)
# ifdef GDAL_EXPORTS
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change the current behaviour of GDAL. Is it needed ? If such, then please revert the logic with for example #ifndef GDAL_IMPORTS

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, GDAL_EXPORTS is a define that is specific to our build environment.

However, I believe that the introduction of GDAL_IMPORTS would break application code.

Instead, I think that DLLBUILD, configured in nmake.opt should be used. Hence, in total:

#ifndef CPL_DLL
#if defined(_MSC_VER)
#  ifdef DLLBUILD
#    define CPL_DLL __declspec(dllexport)
#  elif !defined(CPL_DISABLE_DLL)
#    define CPL_DLL __declspec(dllimport)
#  else
#    define CPL_DLL
#  endif
#else
#  if defined(USE_GCC_VISIBILITY_FLAG)
#    define CPL_DLL     __attribute__ ((visibility("default")))
#  else
#    define CPL_DLL
#  endif
#endif
#endif

Is that fine?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you just revert that change ? I think for your purposes adding -DCPL_DLL= or -DCPL_DLL=__declspec(dllimport) to your nmake.opt should be enough

@rouault
Copy link
Member

rouault commented Mar 12, 2018

Doxygen warning : "/home/travis/build/OSGeo/gdal/gdal/port/cpl_string.h:359: warning: Member CPLStringT (define) of file cpl_string.h is not documented"

I'd suggest not to document it actually with

/*! @cond Doxygen_Suppress */
#  define CPLStringT CPLString
/*! @endcond */

@rouault
Copy link
Member

rouault commented Mar 12, 2018

@@ -334,20 +334,48 @@ extern "C++"
#endif

//! Convenient string class based on std::string.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please move that "//! Convenient string class based on std::string" Doxygen line just before "class CPL_DLL CPLString : public std::string", so as to fix the "cpl_string.h:366: warning: Compound CPLString is not documented." cppcheck error

kwrobot pushed a commit to aashish24/gdal-svn that referenced this pull request Mar 15, 2018
…o (patch by opals, fixes #7254, fixes OSGeo/gdal#321)

git-svn-id: https://svn.osgeo.org/gdal/trunk/gdal@41795 f0d54148-0727-0410-94bb-9a71ac55c965
@rouault rouault closed this in 01cd6ad Mar 15, 2018
rouault pushed a commit that referenced this pull request May 22, 2018
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.

None yet

2 participants