refactor(api): provide a temporary copy solution for nvim_call_atomic

Make the copy_object() family accept an optional arena. More than
half of the callsites should be refactored to use an arena later
anyway.
This commit is contained in:
bfredl
2022-08-23 13:52:09 +02:00
parent bcf5ee328e
commit 7784dc9e0d
17 changed files with 71 additions and 64 deletions

View File

@@ -887,12 +887,12 @@ static bool check_autocmd_string_array(Array arr, char *k, Error *err)
static bool unpack_string_or_array(Array *array, Object *v, char *k, bool required, Error *err) static bool unpack_string_or_array(Array *array, Object *v, char *k, bool required, Error *err)
{ {
if (v->type == kObjectTypeString) { if (v->type == kObjectTypeString) {
ADD(*array, copy_object(*v)); ADD(*array, copy_object(*v, NULL));
} else if (v->type == kObjectTypeArray) { } else if (v->type == kObjectTypeArray) {
if (!check_autocmd_string_array(v->data.array, k, err)) { if (!check_autocmd_string_array(v->data.array, k, err)) {
return false; return false;
} }
*array = copy_array(v->data.array); *array = copy_array(v->data.array, NULL);
} else { } else {
if (required) { if (required) {
api_set_error(err, api_set_error(err,

View File

@@ -51,7 +51,7 @@ Integer nvim_create_namespace(String name)
} }
id = next_namespace_id++; id = next_namespace_id++;
if (name.size > 0) { if (name.size > 0) {
String name_alloc = copy_string(name); String name_alloc = copy_string(name, NULL);
map_put(String, handle_T)(&namespace_ids, name_alloc, id); map_put(String, handle_T)(&namespace_ids, name_alloc, id);
} }
return (Integer)id; return (Integer)id;

View File

@@ -618,6 +618,7 @@ void api_clear_error(Error *value)
value->type = kErrorTypeNone; value->type = kErrorTypeNone;
} }
/// @returns a shared value. caller must not modify it!
Dictionary api_metadata(void) Dictionary api_metadata(void)
{ {
static Dictionary metadata = ARRAY_DICT_INIT; static Dictionary metadata = ARRAY_DICT_INIT;
@@ -630,7 +631,7 @@ Dictionary api_metadata(void)
init_type_metadata(&metadata); init_type_metadata(&metadata);
} }
return copy_object(DICTIONARY_OBJ(metadata)).data.dictionary; return metadata;
} }
static void init_function_metadata(Dictionary *metadata) static void init_function_metadata(Dictionary *metadata)
@@ -715,36 +716,40 @@ static void init_type_metadata(Dictionary *metadata)
PUT(*metadata, "types", DICTIONARY_OBJ(types)); PUT(*metadata, "types", DICTIONARY_OBJ(types));
} }
String copy_string(String str) // all the copy_[object] functions allow arena=NULL,
// then global allocations are used, and the resulting object
// should be freed with an api_free_[object] function
String copy_string(String str, Arena *arena)
{ {
if (str.data != NULL) { if (str.data != NULL) {
return (String){ .data = xmemdupz(str.data, str.size), .size = str.size }; return (String){ .data = arena_memdupz(arena, str.data, str.size), .size = str.size };
} else { } else {
return (String)STRING_INIT; return (String)STRING_INIT;
} }
} }
Array copy_array(Array array) Array copy_array(Array array, Arena *arena)
{ {
Array rv = ARRAY_DICT_INIT; Array rv = arena_array(arena, array.size);
for (size_t i = 0; i < array.size; i++) { for (size_t i = 0; i < array.size; i++) {
ADD(rv, copy_object(array.items[i])); ADD(rv, copy_object(array.items[i], arena));
} }
return rv; return rv;
} }
Dictionary copy_dictionary(Dictionary dict) Dictionary copy_dictionary(Dictionary dict, Arena *arena)
{ {
Dictionary rv = ARRAY_DICT_INIT; Dictionary rv = arena_dict(arena, dict.size);
for (size_t i = 0; i < dict.size; i++) { for (size_t i = 0; i < dict.size; i++) {
KeyValuePair item = dict.items[i]; KeyValuePair item = dict.items[i];
PUT(rv, item.key.data, copy_object(item.value)); PUT_C(rv, copy_string(item.key, arena).data, copy_object(item.value, arena));
} }
return rv; return rv;
} }
/// Creates a deep clone of an object /// Creates a deep clone of an object
Object copy_object(Object obj) Object copy_object(Object obj, Arena *arena)
{ {
switch (obj.type) { switch (obj.type) {
case kObjectTypeBuffer: case kObjectTypeBuffer:
@@ -757,13 +762,13 @@ Object copy_object(Object obj)
return obj; return obj;
case kObjectTypeString: case kObjectTypeString:
return STRING_OBJ(copy_string(obj.data.string)); return STRING_OBJ(copy_string(obj.data.string, arena));
case kObjectTypeArray: case kObjectTypeArray:
return ARRAY_OBJ(copy_array(obj.data.array)); return ARRAY_OBJ(copy_array(obj.data.array, arena));
case kObjectTypeDictionary: case kObjectTypeDictionary:
return DICTIONARY_OBJ(copy_dictionary(obj.data.dictionary)); return DICTIONARY_OBJ(copy_dictionary(obj.data.dictionary, arena));
case kObjectTypeLuaRef: case kObjectTypeLuaRef:
return LUAREF_OBJ(api_new_luaref(obj.data.luaref)); return LUAREF_OBJ(api_new_luaref(obj.data.luaref));
@@ -844,7 +849,7 @@ HlMessage parse_hl_msg(Array chunks, Error *err)
goto free_exit; goto free_exit;
} }
String str = copy_string(chunk.items[0].data.string); String str = copy_string(chunk.items[0].data.string, NULL);
int attr = 0; int attr = 0;
if (chunk.size == 2) { if (chunk.size == 2) {

View File

@@ -965,7 +965,7 @@ static Array translate_contents(UI *ui, Array contents)
} else { } else {
ADD(new_item, DICTIONARY_OBJ((Dictionary)ARRAY_DICT_INIT)); ADD(new_item, DICTIONARY_OBJ((Dictionary)ARRAY_DICT_INIT));
} }
ADD(new_item, copy_object(item.items[1])); ADD(new_item, copy_object(item.items[1], NULL));
ADD(new_contents, ARRAY_OBJ(new_item)); ADD(new_contents, ARRAY_OBJ(new_item));
} }
return new_contents; return new_contents;
@@ -978,7 +978,7 @@ static Array translate_firstarg(UI *ui, Array args)
ADD(new_args, ARRAY_OBJ(translate_contents(ui, contents))); ADD(new_args, ARRAY_OBJ(translate_contents(ui, contents)));
for (size_t i = 1; i < args.size; i++) { for (size_t i = 1; i < args.size; i++) {
ADD(new_args, copy_object(args.items[i])); ADD(new_args, copy_object(args.items[i], NULL));
} }
return new_args; return new_args;
} }
@@ -1024,7 +1024,7 @@ static void remote_ui_event(UI *ui, char *name, Array args)
Array items = args.items[0].data.array; Array items = args.items[0].data.array;
Array new_items = ARRAY_DICT_INIT; Array new_items = ARRAY_DICT_INIT;
for (size_t i = 0; i < items.size; i++) { for (size_t i = 0; i < items.size; i++) {
ADD(new_items, copy_object(items.items[i].data.array.items[0])); ADD(new_items, copy_object(items.items[i].data.array.items[0], NULL));
} }
ADD_C(new_args, ARRAY_OBJ(new_items)); ADD_C(new_args, ARRAY_OBJ(new_items));
push_call(ui, "wildmenu_show", new_args); push_call(ui, "wildmenu_show", new_args);

View File

@@ -1479,14 +1479,14 @@ void nvim_del_keymap(uint64_t channel_id, String mode, String lhs, Error *err)
/// 1 is the |api-metadata| map (Dictionary). /// 1 is the |api-metadata| map (Dictionary).
/// ///
/// @returns 2-tuple [{channel-id}, {api-metadata}] /// @returns 2-tuple [{channel-id}, {api-metadata}]
Array nvim_get_api_info(uint64_t channel_id) Array nvim_get_api_info(uint64_t channel_id, Arena *arena)
FUNC_API_SINCE(1) FUNC_API_FAST FUNC_API_REMOTE_ONLY FUNC_API_SINCE(1) FUNC_API_FAST FUNC_API_REMOTE_ONLY
{ {
Array rv = ARRAY_DICT_INIT; Array rv = arena_array(arena, 2);
assert(channel_id <= INT64_MAX); assert(channel_id <= INT64_MAX);
ADD(rv, INTEGER_OBJ((int64_t)channel_id)); ADD_C(rv, INTEGER_OBJ((int64_t)channel_id));
ADD(rv, DICTIONARY_OBJ(api_metadata())); ADD_C(rv, DICTIONARY_OBJ(api_metadata()));
return rv; return rv;
} }
@@ -1545,9 +1545,9 @@ void nvim_set_client_info(uint64_t channel_id, String name, Dictionary version,
FUNC_API_SINCE(4) FUNC_API_REMOTE_ONLY FUNC_API_SINCE(4) FUNC_API_REMOTE_ONLY
{ {
Dictionary info = ARRAY_DICT_INIT; Dictionary info = ARRAY_DICT_INIT;
PUT(info, "name", copy_object(STRING_OBJ(name))); PUT(info, "name", copy_object(STRING_OBJ(name), NULL));
version = copy_dictionary(version); version = copy_dictionary(version, NULL);
bool has_major = false; bool has_major = false;
for (size_t i = 0; i < version.size; i++) { for (size_t i = 0; i < version.size; i++) {
if (strequal(version.items[i].key.data, "major")) { if (strequal(version.items[i].key.data, "major")) {
@@ -1560,9 +1560,9 @@ void nvim_set_client_info(uint64_t channel_id, String name, Dictionary version,
} }
PUT(info, "version", DICTIONARY_OBJ(version)); PUT(info, "version", DICTIONARY_OBJ(version));
PUT(info, "type", copy_object(STRING_OBJ(type))); PUT(info, "type", copy_object(STRING_OBJ(type), NULL));
PUT(info, "methods", DICTIONARY_OBJ(copy_dictionary(methods))); PUT(info, "methods", DICTIONARY_OBJ(copy_dictionary(methods, NULL)));
PUT(info, "attributes", DICTIONARY_OBJ(copy_dictionary(attributes))); PUT(info, "attributes", DICTIONARY_OBJ(copy_dictionary(attributes, NULL)));
rpc_set_client_info(channel_id, info); rpc_set_client_info(channel_id, info);
} }
@@ -1642,21 +1642,21 @@ Array nvim_call_atomic(uint64_t channel_id, Array calls, Arena *arena, Error *er
api_set_error(err, api_set_error(err,
kErrorTypeValidation, kErrorTypeValidation,
"Items in calls array must be arrays"); "Items in calls array must be arrays");
goto validation_error; goto theend;
} }
Array call = calls.items[i].data.array; Array call = calls.items[i].data.array;
if (call.size != 2) { if (call.size != 2) {
api_set_error(err, api_set_error(err,
kErrorTypeValidation, kErrorTypeValidation,
"Items in calls array must be arrays of size 2"); "Items in calls array must be arrays of size 2");
goto validation_error; goto theend;
} }
if (call.items[0].type != kObjectTypeString) { if (call.items[0].type != kObjectTypeString) {
api_set_error(err, api_set_error(err,
kErrorTypeValidation, kErrorTypeValidation,
"Name must be String"); "Name must be String");
goto validation_error; goto theend;
} }
String name = call.items[0].data.string; String name = call.items[0].data.string;
@@ -1664,7 +1664,7 @@ Array nvim_call_atomic(uint64_t channel_id, Array calls, Arena *arena, Error *er
api_set_error(err, api_set_error(err,
kErrorTypeValidation, kErrorTypeValidation,
"Args must be Array"); "Args must be Array");
goto validation_error; goto theend;
} }
Array args = call.items[1].data.array; Array args = call.items[1].data.array;
@@ -1682,11 +1682,13 @@ Array nvim_call_atomic(uint64_t channel_id, Array calls, Arena *arena, Error *er
// error handled after loop // error handled after loop
break; break;
} }
if (!handler.arena_return && result.type != kObjectTypeNil) { // TODO(bfredl): wastefull copy. It could be avoided to encoding to msgpack
// TODO: fix to not leak memory as fuck // directly here. But `result` might become invalid when next api function
// is called in the loop.
ADD_C(results, copy_object(result, arena));
if (!handler.arena_return) {
api_free_object(result);
} }
ADD_C(results, result);
} }
ADD_C(rv, ARRAY_OBJ(results)); ADD_C(rv, ARRAY_OBJ(results));
@@ -1694,14 +1696,12 @@ Array nvim_call_atomic(uint64_t channel_id, Array calls, Arena *arena, Error *er
Array errval = arena_array(arena, 3); Array errval = arena_array(arena, 3);
ADD_C(errval, INTEGER_OBJ((Integer)i)); ADD_C(errval, INTEGER_OBJ((Integer)i));
ADD_C(errval, INTEGER_OBJ(nested_error.type)); ADD_C(errval, INTEGER_OBJ(nested_error.type));
ADD_C(errval, STRING_OBJ(cstr_to_string(nested_error.msg))); // TODO ADD_C(errval, STRING_OBJ(copy_string(cstr_as_string(nested_error.msg), arena)));
ADD_C(rv, ARRAY_OBJ(errval)); ADD_C(rv, ARRAY_OBJ(errval));
} else { } else {
ADD_C(rv, NIL); ADD_C(rv, NIL);
} }
goto theend;
validation_error:
theend: theend:
api_clear_error(&nested_error); api_clear_error(&nested_error);
return rv; return rv;
@@ -1754,7 +1754,7 @@ static void write_msg(String message, bool to_err)
/// @return its argument. /// @return its argument.
Object nvim__id(Object obj) Object nvim__id(Object obj)
{ {
return copy_object(obj); return copy_object(obj, NULL);
} }
/// Returns array given as argument. /// Returns array given as argument.
@@ -1767,7 +1767,7 @@ Object nvim__id(Object obj)
/// @return its argument. /// @return its argument.
Array nvim__id_array(Array arr) Array nvim__id_array(Array arr)
{ {
return copy_object(ARRAY_OBJ(arr)).data.array; return copy_array(arr, NULL);
} }
/// Returns dictionary given as argument. /// Returns dictionary given as argument.
@@ -1780,7 +1780,7 @@ Array nvim__id_array(Array arr)
/// @return its argument. /// @return its argument.
Dictionary nvim__id_dictionary(Dictionary dct) Dictionary nvim__id_dictionary(Dictionary dct)
{ {
return copy_object(DICTIONARY_OBJ(dct)).data.dictionary; return copy_dictionary(dct, NULL);
} }
/// Returns floating-point value given as argument. /// Returns floating-point value given as argument.

View File

@@ -2051,7 +2051,7 @@ static bool call_autocmd_callback(const AutoCmd *ac, const AutoPatCmd *apc)
PUT(data, "buf", INTEGER_OBJ(autocmd_bufnr)); PUT(data, "buf", INTEGER_OBJ(autocmd_bufnr));
if (apc->data) { if (apc->data) {
PUT(data, "data", copy_object(*apc->data)); PUT(data, "data", copy_object(*apc->data, NULL));
} }
int group = apc->curpat->group; int group = apc->curpat->group;

View File

@@ -345,7 +345,7 @@ Dictionary ctx_to_dict(Context *ctx)
PUT(rv, "jumps", ARRAY_OBJ(sbuf_to_array(ctx->jumps))); PUT(rv, "jumps", ARRAY_OBJ(sbuf_to_array(ctx->jumps)));
PUT(rv, "bufs", ARRAY_OBJ(sbuf_to_array(ctx->bufs))); PUT(rv, "bufs", ARRAY_OBJ(sbuf_to_array(ctx->bufs)));
PUT(rv, "gvars", ARRAY_OBJ(sbuf_to_array(ctx->gvars))); PUT(rv, "gvars", ARRAY_OBJ(sbuf_to_array(ctx->gvars)));
PUT(rv, "funcs", ARRAY_OBJ(copy_array(ctx->funcs))); PUT(rv, "funcs", ARRAY_OBJ(copy_array(ctx->funcs, NULL)));
return rv; return rv;
} }
@@ -381,7 +381,7 @@ int ctx_from_dict(Dictionary dict, Context *ctx)
ctx->gvars = array_to_sbuf(item.value.data.array); ctx->gvars = array_to_sbuf(item.value.data.array);
} else if (strequal(item.key.data, "funcs")) { } else if (strequal(item.key.data, "funcs")) {
types |= kCtxFuncs; types |= kCtxFuncs;
ctx->funcs = copy_object(item.value).data.array; ctx->funcs = copy_object(item.value, NULL).data.array;
} }
} }

View File

@@ -359,7 +359,6 @@ static void f_api_info(typval_T *argvars, typval_T *rettv, EvalFuncData fptr)
{ {
Dictionary metadata = api_metadata(); Dictionary metadata = api_metadata();
(void)object_to_vim(DICTIONARY_OBJ(metadata), rettv, NULL); (void)object_to_vim(DICTIONARY_OBJ(metadata), rettv, NULL);
api_free_dictionary(metadata);
} }
/// "append(lnum, string/list)" function /// "append(lnum, string/list)" function

View File

@@ -125,7 +125,7 @@ for i = 1, #events do
local param = ev.parameters[j] local param = ev.parameters[j]
local copy = 'copy_'..param[2] local copy = 'copy_'..param[2]
if param[1] == 'String' then if param[1] == 'String' then
send = send..' String copy_'..param[2]..' = copy_string('..param[2]..');\n' send = send..' String copy_'..param[2]..' = copy_string('..param[2]..', NULL);\n'
argv = argv..', '..copy..'.data, INT2PTR('..copy..'.size)' argv = argv..', '..copy..'.data, INT2PTR('..copy..'.size)'
recv = (recv..' String '..param[2].. recv = (recv..' String '..param[2]..
' = (String){.data = argv['..argc..'],'.. ' = (String){.data = argv['..argc..'],'..
@@ -134,7 +134,7 @@ for i = 1, #events do
recv_cleanup = recv_cleanup..' api_free_string('..param[2]..');\n' recv_cleanup = recv_cleanup..' api_free_string('..param[2]..');\n'
argc = argc+2 argc = argc+2
elseif param[1] == 'Array' then elseif param[1] == 'Array' then
send = send..' Array '..copy..' = copy_array('..param[2]..');\n' send = send..' Array '..copy..' = copy_array('..param[2]..', NULL);\n'
argv = argv..', '..copy..'.items, INT2PTR('..copy..'.size)' argv = argv..', '..copy..'.items, INT2PTR('..copy..'.size)'
recv = (recv..' Array '..param[2].. recv = (recv..' Array '..param[2]..
' = (Array){.items = argv['..argc..'],'.. ' = (Array){.items = argv['..argc..'],'..
@@ -144,7 +144,7 @@ for i = 1, #events do
argc = argc+2 argc = argc+2
elseif param[1] == 'Object' then elseif param[1] == 'Object' then
send = send..' Object *'..copy..' = xmalloc(sizeof(Object));\n' send = send..' Object *'..copy..' = xmalloc(sizeof(Object));\n'
send = send..' *'..copy..' = copy_object('..param[2]..');\n' send = send..' *'..copy..' = copy_object('..param[2]..', NULL);\n'
argv = argv..', '..copy argv = argv..', '..copy
recv = recv..' Object '..param[2]..' = *(Object *)argv['..argc..'];\n' recv = recv..' Object '..param[2]..' = *(Object *)argv['..argc..'];\n'
recv_argv = recv_argv..', '..param[2] recv_argv = recv_argv..', '..param[2]

View File

@@ -903,7 +903,7 @@ Dictionary hlattrs2dict(Dictionary *hl_alloc, HlAttrs ae, bool use_rgb)
*hl_alloc = hl; *hl_alloc = hl;
return hl; return hl;
} else { } else {
Dictionary allocated = copy_dictionary(hl); Dictionary allocated = copy_dictionary(hl, NULL);
kv_destroy(hl); kv_destroy(hl);
return allocated; return allocated;
} }

View File

@@ -575,9 +575,13 @@ void alloc_block(Arena *arena)
blk->prev = prev_blk; blk->prev = prev_blk;
} }
/// @param arena if NULL, do a global allocation. caller must then free the value!
/// @param size if zero, will still return a non-null pointer, but not a unique one /// @param size if zero, will still return a non-null pointer, but not a unique one
void *arena_alloc(Arena *arena, size_t size, bool align) void *arena_alloc(Arena *arena, size_t size, bool align)
{ {
if (!arena) {
return xmalloc(size);
}
if (align) { if (align) {
arena->pos = (arena->pos + (ARENA_ALIGN - 1)) & ~(ARENA_ALIGN - 1); arena->pos = (arena->pos + (ARENA_ALIGN - 1)) & ~(ARENA_ALIGN - 1);
} }

View File

@@ -1086,7 +1086,7 @@ void ex_messages(void *const eap_p)
HlMessageChunk chunk = kv_A(p->multiattr, i); HlMessageChunk chunk = kv_A(p->multiattr, i);
Array content_entry = ARRAY_DICT_INIT; Array content_entry = ARRAY_DICT_INIT;
ADD(content_entry, INTEGER_OBJ(chunk.attr)); ADD(content_entry, INTEGER_OBJ(chunk.attr));
ADD(content_entry, STRING_OBJ(copy_string(chunk.text))); ADD(content_entry, STRING_OBJ(copy_string(chunk.text, NULL)));
ADD(content, ARRAY_OBJ(content_entry)); ADD(content, ARRAY_OBJ(content_entry));
} }
} else if (p->msg && p->msg[0]) { } else if (p->msg && p->msg[0]) {

View File

@@ -644,7 +644,7 @@ void rpc_set_client_info(uint64_t id, Dictionary info)
Dictionary rpc_client_info(Channel *chan) Dictionary rpc_client_info(Channel *chan)
{ {
return copy_dictionary(chan->rpc.info); return copy_dictionary(chan->rpc.info, NULL);
} }
const char *rpc_client_name(Channel *chan) const char *rpc_client_name(Channel *chan)

View File

@@ -233,7 +233,7 @@ static void tinput_wait_enqueue(void **argv)
if (ui_client_channel_id) { if (ui_client_channel_id) {
Array args = ARRAY_DICT_INIT; Array args = ARRAY_DICT_INIT;
Error err = ERROR_INIT; Error err = ERROR_INIT;
ADD(args, STRING_OBJ(copy_string(keys))); ADD(args, STRING_OBJ(copy_string(keys, NULL)));
// TODO(bfredl): could be non-blocking now with paste? // TODO(bfredl): could be non-blocking now with paste?
ArenaMem res_mem = NULL; ArenaMem res_mem = NULL;
Object result = rpc_send_call(ui_client_channel_id, "nvim_input", args, &res_mem, &err); Object result = rpc_send_call(ui_client_channel_id, "nvim_input", args, &res_mem, &err);

View File

@@ -33,7 +33,6 @@ typedef union {
void *nullptr; void *nullptr;
} EvalFuncData; } EvalFuncData;
typedef handle_T NS; typedef handle_T NS;
typedef struct expand expand_T; typedef struct expand expand_T;

View File

@@ -194,9 +194,9 @@ static void ui_bridge_suspend_event(void **argv)
static void ui_bridge_option_set(UI *ui, String name, Object value) static void ui_bridge_option_set(UI *ui, String name, Object value)
{ {
String copy_name = copy_string(name); String copy_name = copy_string(name, NULL);
Object *copy_value = xmalloc(sizeof(Object)); Object *copy_value = xmalloc(sizeof(Object));
*copy_value = copy_object(value); *copy_value = copy_object(value, NULL);
UI_BRIDGE_CALL(ui, option_set, 4, ui, copy_name.data, UI_BRIDGE_CALL(ui, option_set, 4, ui, copy_name.data,
INT2PTR(copy_name.size), copy_value); INT2PTR(copy_name.size), copy_value);
// TODO(bfredl): when/if TUI/bridge teardown is refactored to use events, the // TODO(bfredl): when/if TUI/bridge teardown is refactored to use events, the