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/geojson
gdal/ogr/ogrsf_frmts/gmlas
gdal/ogr/ogrsf_frmts/elastic
gdal/ogr/ogrsf_frmts/couchdb
gdal/ogr
gdal/frmts/rda
gdal/frmts/arg
gdal/frmts/mbtiles
gdal/frmts/plmosaic
gdal/frmts/pds

It's big work to replace json-c in all code smoothly.

Best regards,
    Dmitry

05.01.2018 19:05, 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 already done well by
the lib.

On Fri, Jan 5, 2018 at 7:54 AM, Kurt Schwehr <schw...@gmail.com> wrote:

+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 general about GDAL for a long time.

* 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 bothered by subclassing of
std::string as CPLString.  After reading large amounts of C++ code, I think
it adds more confusion than it ever helps over having clean free
functions.  Interop and analysis with CPLString's is no fun.
   https://stackoverflow.com/questions/6006860/why-should-
one-not-derive-from-c-std-string-class

-kurt

On Fri, Jan 5, 2018 at 7:44 AM, Sean Gillies <s...@mapbox.com> wrote:

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
run the benchmarks, but json-c compares pretty poorly to others in
https://github.com/miloyip/nativejson-benchmark.


On Wed, Jan 3, 2018 at 12:04 PM, Dmitry Baryshnikov <bishop....@gmail.com
wrote:
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. The web or disk json documents reading chunk by chunk with progress
indication also added.

In future, the json-c can be easily switch to something other without
breaking the existing code.

The CPLJSONDocument/CPLJSONObject/CPLJSONArray usage examples can be
found in frmts/pds driver and c++ unit test in autotest/cpp/test_cpl.cpp.

Is this ready to merge into the trunk? Any objections?

Best regards,
     Dmitry



_______________________________________________
gdal-dev mailing list
gdal-dev@lists.osgeo.org
https://lists.osgeo.org/mailman/listinfo/gdal-dev

Reply via email to