From 98521267d508d5bcfbc333d0596843dd691871a2 Mon Sep 17 00:00:00 2001 From: Jose M Perez Date: Tue, 7 Apr 2020 12:04:22 +0200 Subject: [PATCH] Manage copy_strings with max_levels option again --- c_src/decoder.c | 8 +++++- c_src/jiffy.h | 1 + c_src/wrapper.c | 13 ++++++++- test/jiffy_18_partials_tests.erl | 46 +++++++++++++++++++------------- 4 files changed, 47 insertions(+), 21 deletions(-) diff --git a/c_src/decoder.c b/c_src/decoder.c index 7988098..a96fbff 100644 --- a/c_src/decoder.c +++ b/c_src/decoder.c @@ -208,7 +208,13 @@ static int inline level_decrease(Decoder* d, ERL_NIF_TERM* value) { if (d->max_levels && d->max_levels == --d->current_depth) { // Only builds term in threshold - *value = wrap_enif_make_sub_binary(d->env, d->arg, d->level_start, (d->i - d->level_start + 1)); + unsigned ulen = d->i - d->level_start + 1; + if(!d->copy_strings) { + *value = wrap_enif_make_sub_binary(d->env, d->arg, d->level_start, ulen); + } else { + char* chrbuf = wrap_enif_make_new_binary(d->env, ulen, value); + memcpy(chrbuf, &(d->p[d->level_start]), ulen); + } return 1; } return 0; diff --git a/c_src/jiffy.h b/c_src/jiffy.h index 5902480..b0a1889 100644 --- a/c_src/jiffy.h +++ b/c_src/jiffy.h @@ -88,6 +88,7 @@ int unicode_from_pair(int hi, int lo); int unicode_uescape(int c, unsigned char* buf); int double_to_shortest(unsigned char *buf, size_t size, size_t* len, double val); +char* wrap_enif_make_new_binary(ErlNifEnv* env, size_t size, ERL_NIF_TERM* termp); ERL_NIF_TERM wrap_enif_make_sub_binary(ErlNifEnv* env, ERL_NIF_TERM bin_term, size_t pos, size_t size); int unwrap(ErlNifEnv* env, ERL_NIF_TERM wrapper_resource, ERL_NIF_TERM* bin_term_p); diff --git a/c_src/wrapper.c b/c_src/wrapper.c index 13c9511..3a77e26 100644 --- a/c_src/wrapper.c +++ b/c_src/wrapper.c @@ -49,11 +49,22 @@ ERL_NIF_TERM wrap_enif_make_sub_binary(ErlNifEnv* env, ERL_NIF_TERM bin_term, size_t pos, size_t size) { ErlNifEnv* process_independent_env = enif_alloc_env(); - // sub_bin must be created in the same env as the parent binary and then copied + // sub_bin must be created in the same env as the parent binary and then + // copied, segfaults sometimes otherwise ERL_NIF_TERM sub_bin = enif_make_sub_binary(env, bin_term, pos, size); return wrap_new(env, process_independent_env, enif_make_copy(process_independent_env, sub_bin)); } +char* +wrap_enif_make_new_binary(ErlNifEnv* env, size_t size, ERL_NIF_TERM* termp) +{ + ErlNifEnv* process_independent_env = enif_alloc_env(); + ERL_NIF_TERM bin; + char* chrbuf = (char*) enif_make_new_binary(process_independent_env, size, &bin); + *termp = wrap_new(env, process_independent_env, bin); + return chrbuf; +} + int unwrap(ErlNifEnv* env, ERL_NIF_TERM wrapper_resource, ERL_NIF_TERM* bin_term_p) { diff --git a/test/jiffy_18_partials_tests.erl b/test/jiffy_18_partials_tests.erl index abf2883..29fba69 100644 --- a/test/jiffy_18_partials_tests.erl +++ b/test/jiffy_18_partials_tests.erl @@ -8,33 +8,30 @@ decode_levels_test_() -> MaxOptMaxLevels = 4, {"Test max_levels", lists:map(fun(Json) -> - [ - begin - EJson = jiffy:decode(Json, [{max_levels, MaxLevels} | Opts]), - FullEJson = to_full_json(EJson, MaxLevels, Opts), - ?_assertEqual(jiffy:decode(Json, Opts), FullEJson) + [begin + EJson = jiffy:decode(Json, [{max_levels, MaxLevels} | Opts]), + FullEJson = to_full_json(EJson, MaxLevels, Opts), + ?_assertEqual(jiffy:decode(Json, Opts), FullEJson) end || MaxLevels <- lists:seq(1, MaxOptMaxLevels), Opts <- generate_options_groups()] end, jsons())}. encode_resources_test_() -> {"Test encode resources", lists:map(fun(Json) -> - [ - begin - EJsonWithResources = jiffy:decode(Json, [{max_levels, 1} | Opts]), - JsonFromResources = jiffy:encode(EJsonWithResources), - ?_assertEqual(jiffy:decode(Json, Opts), jiffy:decode(JsonFromResources, Opts)) + [begin + EJsonWithResources = jiffy:decode(Json, [{max_levels, 1} | Opts]), + JsonFromResources = jiffy:encode(EJsonWithResources), + ?_assertEqual(jiffy:decode(Json, Opts), jiffy:decode(JsonFromResources, Opts)) end || Opts <- generate_options_groups()] end, jsons())}. encode_partials_test_() -> {"Test encode partials", lists:map(fun(Json) -> - [ - begin - EJson = jiffy:decode(Json, Opts), - PartialResource = jiffy:encode(EJson, [partial]), - true = is_reference(PartialResource), - PartialIOData = jiffy:encode(PartialResource), - ?_assertEqual(EJson, jiffy:decode(PartialIOData, Opts)) + [begin + EJson = jiffy:decode(Json, Opts), + PartialResource = jiffy:encode(EJson, [partial]), + true = is_reference(PartialResource), + PartialIOData = jiffy:encode(PartialResource), + ?_assertEqual(EJson, jiffy:decode(PartialIOData, Opts)) end || Opts <- generate_options_groups()] end, jsons())}. @@ -49,9 +46,9 @@ jsons() -> -ifndef(JIFFY_NO_MAPS). -generate_options_groups() -> generate_options_groups([return_maps]). +generate_options_groups() -> generate_options_groups([copy_strings, return_maps]). -else. -generate_options_groups() -> generate_options_groups([]). +generate_options_groups() -> generate_options_groups([copy_strings]). -endif. generate_options_groups(AvailableOptions) -> @@ -69,6 +66,17 @@ to_full_json(_Val, Depth, MaxDepth, _DecodeOptions) when Depth > MaxDepth -> to_full_json(PartialResource, Depth, MaxDepth, DecodeOptions) when is_reference(PartialResource) -> MaxDepth = Depth, IOData = jiffy:encode(PartialResource), + [begin + ByteSize = byte_size(ValueBin), + case lists:member(copy_strings, DecodeOptions) of + true -> + ByteSize = binary:referenced_byte_size(ValueBin); + _ -> + % With small binaries, the copies between environments involve a + % full copy if the binary is small enough (thus the =) + true = ByteSize =< binary:referenced_byte_size(ValueBin) + end + end || ValueBin <- lists:flatten(IOData)], jiffy:decode(IOData, DecodeOptions); to_full_json({Pairs}, Depth, MaxDepth, DecodeOptions) when is_list(Pairs) -> {[{K, to_full_json(V, Depth+1, MaxDepth, DecodeOptions)} || {K, V} <- Pairs]};