Re: [gdal-dev] CPLJSONDocument

2018-01-09 Thread Dmitry Baryshnikov
Hi, I changed CPLJSONObject/CPLJSONArray  methods string parameters to std::string and merged last trunk changes. Still leave 3 methods with const char* to simplify code because according to standard conversion sequence, methods with input (const char*, const char*) parameters executes metho

Re: [gdal-dev] CPLJSONDocument

2018-01-06 Thread Kurt Schwehr
Andrew, My take: GDAL is a not a place to look for good modern C++ code. And there is a of old style C++ baked into the APIs, so drivers are particularly tough. You can look for use of unique_ptr for some code that is heading in that direction. The other general problem with C++ code in GDAL i

Re: [gdal-dev] CPLJSONDocument

2018-01-06 Thread Andrew C Aitchison
On Fri, 5 Jan 2018, Kurt Schwehr wrote: * Passing char *psz yada all over the place in pure C++ code. A const std::string is usually not a noticeable expense and is a lot safer * CPLString when std::string will do just fine. And we can write free functions to operate on strings. I'm generally

Re: [gdal-dev] CPLJSONDocument

2018-01-05 Thread Mateusz Loskot
On 5 January 2018 at 21:43, Kurt Schwehr wrote: > My preference (and not speaking for the gdal community) for C++ classes > would be: > > 1. replace const char * -> const std::string & > 2. replace CPLString -> std::string +1 > Then in about 2022, we can see about std::string_view :( +1 Best r

Re: [gdal-dev] CPLJSONDocument

2018-01-05 Thread Kurt Schwehr
My preference (and not speaking for the gdal community) for C++ classes would be: 1. replace const char * -> const std::string & 2. replace CPLString -> std::string std::string GetString(const std::string &soName, const std::string &soDefault = "") const; This is where it would be good to get in

Re: [gdal-dev] CPLJSONDocument

2018-01-05 Thread Dmitry Baryshnikov
Hi Kurt, Can you explain what should be done in PR? Do you mean to replace all const char* to? 1. const char* -> const CPLString & const char *GetString(const char *pszName, const char *pszDefault = "") const; -> CPLString GetString(const CPLString &soName, const CPLString &soDefa

Re: [gdal-dev] CPLJSONDocument

2018-01-05 Thread Dmitry Baryshnikov
Hi Sean, First of all I agreed with Kurt. Json-c is GDAL internal library and had inspected by fuzzer. Also json-c widely used in GDAL: gdal/gcore gdal/ogr/ogrsf_frmts/carto gdal/ogr/ogrsf_frmts/amigocloud gdal/ogr/ogrsf_frmts/cloudant gdal/ogr/ogrsf_frmts/plscenes gdal/ogr/ogrsf_frmts/geojso

Re: [gdal-dev] CPLJSONDocument

2018-01-05 Thread Kurt Schwehr
I think the more important factors than speed for a C++ lib are: security, stability, maintainability, and memory usage (low stack usage and constrainable heap). I really don't want to have us go through yet more piles of fuzzer bugs for a library we depend on. It would be nice to have that alrea

Re: [gdal-dev] CPLJSONDocument

2018-01-05 Thread Kurt Schwehr
+1 for wrapping the old C code in some cleaner abstractions! But +10 for switching to a from the ground up C++ JSON library unless there are clear reasons for a core C library (I don't think there are) If we are talking about this kind of code, there are several things that have bugged me in gene

Re: [gdal-dev] CPLJSONDocument

2018-01-05 Thread Sean Gillies
Hi Dmitry, I scanned the PR and it seems reasonable to me. I'm barely a C++ programmer at all and it's clear to me, more clear than before. That said, I'm not a fan of wrapping things that could be replaced. Have you looked into whether a more performant C++ JSON library could be used? I haven't r

Re: [gdal-dev] CPLJSONDocument

2018-01-05 Thread Even Rouault
Dmitry, > Is this ready to merge into the trunk? Any objections? I did some review of your code. If nobody has extra remarks in the next days, just merge it. Even -- Spatialys - Geospatial professional services http://www.spatialys.com ___ gdal-dev

[gdal-dev] CPLJSONDocument

2018-01-03 Thread Dmitry Baryshnikov
Hi everybody, Happy new year and lot of success in 2018! I would like to discuss my pull request https://github.com/OSGeo/gdal/pull/282 I created a thin wrapper around the json-c library which wide using in GDAL. This is C++ interface which hides C memory management and provides nice API. T