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 NULL and drop SHPLIB_NULLPTR #25

Open
schwehr opened this issue Feb 16, 2021 · 4 comments
Open

Use NULL and drop SHPLIB_NULLPTR #25

schwehr opened this issue Feb 16, 2021 · 4 comments

Comments

@schwehr
Copy link
Member

schwehr commented Feb 16, 2021

SHPLIB_NULLPTR is defined in a number of places. e.g. shpopen.c

#ifdef __cplusplus
#define STATIC_CAST(type,x) static_cast<type>(x)
#define SHPLIB_NULLPTR nullptr
#else
#define STATIC_CAST(type,x) ((type)(x))
#define SHPLIB_NULLPTR NULL
#endif

This doesn't buy much as there is not mechanism in shapelib to build as C++. If the internals are ever converted to C++, then most of the NULLs should be converted to nullptr. The idea of having it both ways just adds complexity. Currently:

grep SHPLIB_NULLPTR *.c | wc -l
245

grep NULL *.c | grep -v SHPLIB_NULLPTR | wc -l
92

@rouault added the cast macros and SHPLIB_NULLPTR in 8ba2778. However, looking at gdal/ogr/ogrsf_frmts/shape, it doesn't currently look like shapelib is being built as C++ in sGDAL

@schwehr schwehr changed the title rop SHPLIB_NULLPTR Use NULL and drop SHPLIB_NULLPTR Feb 16, 2021
@schwehr
Copy link
Member Author

schwehr commented Feb 16, 2021

If there is a need to build shapelib as C++, I think it should just shift to C++ for the library code with a C linkage API. I'd like to hear what drove having it compile as both C and C++.

@schwehr
Copy link
Member Author

schwehr commented Mar 4, 2021

I found one place that uses shapelib as C++:

https://github.com/fulcrumapp/shp-ruby/tree/master/ext/shp

@thbeu
Copy link
Contributor

thbeu commented Mar 9, 2024

SHPLIB_NULLPTR is defined in a number of places. e.g. shpopen.c

It's now exactly in one place: shapefil_private.h

@rouault
Copy link
Member

rouault commented Mar 9, 2024

However, looking at gdal/ogr/ogrsf_frmts/shape, it doesn't currently look like shapelib is being built as C++ in sGDAL

hum, at some point, I certainly did it. Not sure why it didn't materialize in the build system. Anyway now it will per OSGeo/gdal#9436. I've found that building in C++ mode helps finding issues that a C compiler doesn't report.

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