mirror of
https://github.com/felt/tippecanoe.git
synced 2026-10-02 16:35:40 +02:00
Tidy jsonpull call sites flagged in review
- geojson.cpp and attribute.cpp passed key_pool::pool() and set_attribute_accum() a c_str() from a std::string, forcing a needless reconstruction (and truncating at an embedded NUL). Both overloads take std::string, so pass it directly. The geojson.cpp one is the hottest loop in the program. - Replace the hand-maintained counters beside range-for loops in attribute.cpp, main.cpp and tile-join.cpp with indexed loops, since the index is only wanted for error messages. - parse_json_args took json_pull_ptr by value and then copied it, costing two refcount bumps per construction. Move it. - Assert that the parser is still attached where geojson.cpp reads geometry->parser->line. Only json_read results reach it today, but json_read_tree and json_disconnect now clear every parser pointer, so a detached tree would null-deref there instead of tripping an assert. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017KNxyHKasyWrWcvre2yK4r
This commit is contained in:
+4
-4
@@ -55,8 +55,9 @@ void set_attribute_accum(std::unordered_map<std::string, attribute_op> &attribut
|
||||
exit(EXIT_JSON);
|
||||
}
|
||||
|
||||
size_t i = 0;
|
||||
for (const auto &e : o->entries()) {
|
||||
for (size_t i = 0; i < o->entries().size(); i++) {
|
||||
const auto &e = o->entries()[i];
|
||||
|
||||
if (e.key->type != JSON_STRING) {
|
||||
fprintf(stderr, "%s: -E%s: key %zu not a string\n", *argv, arg, i);
|
||||
exit(EXIT_JSON);
|
||||
@@ -66,8 +67,7 @@ void set_attribute_accum(std::unordered_map<std::string, attribute_op> &attribut
|
||||
exit(EXIT_JSON);
|
||||
}
|
||||
|
||||
set_attribute_accum(attribute_accum, e.key->string().c_str(), e.value->string().c_str());
|
||||
i++;
|
||||
set_attribute_accum(attribute_accum, e.key->string(), e.value->string());
|
||||
}
|
||||
|
||||
return;
|
||||
|
||||
+5
-1
@@ -188,7 +188,7 @@ int serialize_geojson_feature(struct serialization_state *sst, json_object *geom
|
||||
if (e.key->type == JSON_STRING) {
|
||||
serial_val sv = stringify_value(e.value.get(), sst->fname, sst->line, feature);
|
||||
|
||||
full_keys.emplace_back(key_pool.pool(e.key->string().c_str()));
|
||||
full_keys.emplace_back(key_pool.pool(e.key->string()));
|
||||
values.push_back(std::move(sv));
|
||||
}
|
||||
}
|
||||
@@ -238,6 +238,10 @@ struct json_serialize_action : json_feature_action {
|
||||
std::string layername;
|
||||
|
||||
int add_feature(json_object *geometry, bool geometrycollection, json_object *properties, json_object *id, json_object *tippecanoe, json_object *feature) {
|
||||
// This only ever receives json_read results from geojson-loop, whose
|
||||
// parser is still attached. json_read_tree / json_disconnect clear
|
||||
// every parser pointer, so a detached tree would null-deref here.
|
||||
assert(geometry->parser != nullptr);
|
||||
sst->line = geometry->parser->line;
|
||||
if (geometrycollection) {
|
||||
int ret = 1;
|
||||
|
||||
+1
-1
@@ -17,7 +17,7 @@ struct parse_json_args {
|
||||
struct serialization_state *sst;
|
||||
|
||||
parse_json_args(json_pull_ptr jp1, int layer1, std::string *layername1, struct serialization_state *sst1)
|
||||
: jp(jp1), layer(layer1), layername(layername1), sst(sst1) {
|
||||
: jp(std::move(jp1)), layer(layer1), layername(layername1), sst(sst1) {
|
||||
}
|
||||
};
|
||||
|
||||
|
||||
@@ -2896,8 +2896,9 @@ void set_attribute_value(const char *arg) {
|
||||
exit(EXIT_JSON);
|
||||
}
|
||||
|
||||
size_t i = 0;
|
||||
for (const auto &e : o->entries()) {
|
||||
for (size_t i = 0; i < o->entries().size(); i++) {
|
||||
const auto &e = o->entries()[i];
|
||||
|
||||
if (e.key->type != JSON_STRING) {
|
||||
fprintf(stderr, "%s: --set-attribute %s: key %zu not a string\n", *av, arg, i);
|
||||
exit(EXIT_JSON);
|
||||
@@ -2905,7 +2906,6 @@ void set_attribute_value(const char *arg) {
|
||||
|
||||
serial_val val = stringify_value(e.value.get(), "json", 1, o.get());
|
||||
set_attributes.emplace(e.key->string(), val);
|
||||
i++;
|
||||
}
|
||||
|
||||
return;
|
||||
|
||||
+3
-3
@@ -973,8 +973,9 @@ void handle_strategies(const unsigned char *s, std::vector<strategy> *st) {
|
||||
for (size_t i = 0; i < o->array().size(); i++) {
|
||||
const json_object_ptr &h = o->array()[i];
|
||||
if (h->type == JSON_HASH) {
|
||||
size_t j = 0;
|
||||
for (const auto &kv : h->entries()) {
|
||||
for (size_t j = 0; j < h->entries().size(); j++) {
|
||||
const auto &kv = h->entries()[j];
|
||||
|
||||
if (kv.key->type != JSON_STRING) {
|
||||
fprintf(stderr, "Key %zu of %zu is not a string: %s\n", j, i, s);
|
||||
} else if (kv.value->type != JSON_NUMBER) {
|
||||
@@ -1005,7 +1006,6 @@ void handle_strategies(const unsigned char *s, std::vector<strategy> *st) {
|
||||
(*st)[i].feature_count += kv.value->number();
|
||||
}
|
||||
}
|
||||
j++;
|
||||
}
|
||||
} else {
|
||||
fprintf(stderr, "Element %zu is not a hash: %s\n", i, s);
|
||||
|
||||
Reference in New Issue
Block a user