diff options
| -rw-r--r-- | scripts/genmsgpack.lua | 58 | ||||
| -rw-r--r-- | src/nvim/api/private/helpers.c | 2 | ||||
| -rw-r--r-- | src/nvim/api/private/helpers.h | 2 | ||||
| -rw-r--r-- | src/nvim/api/vim.c | 21 | ||||
| -rw-r--r-- | src/nvim/msgpack_rpc/helpers.c | 2 | ||||
| -rw-r--r-- | src/nvim/viml/executor/converter.c | 39 | ||||
| -rw-r--r-- | test/functional/api/vim_spec.lua | 11 | ||||
| -rw-r--r-- | test/functional/lua_spec.lua | 51 |
8 files changed, 137 insertions, 49 deletions
diff --git a/scripts/genmsgpack.lua b/scripts/genmsgpack.lua index dd3caab5e4..d47d637548 100644 --- a/scripts/genmsgpack.lua +++ b/scripts/genmsgpack.lua @@ -216,6 +216,14 @@ local function real_type(type) return rv end +local function attr_name(rt) + if rt == 'Float' then + return 'floating' + else + return rt:lower() + end +end + -- start the handler functions. Visit each function metadata to build the -- handler function with code generated for validating arguments and calling to -- the real API. @@ -253,7 +261,7 @@ for i = 1, #functions do output:write('\n '..converted..' = (handle_T)args.items['..(j - 1)..'].data.integer;') else output:write('\n if (args.items['..(j - 1)..'].type == kObjectType'..rt..') {') - output:write('\n '..converted..' = args.items['..(j - 1)..'].data.'..rt:lower()..';') + output:write('\n '..converted..' = args.items['..(j - 1)..'].data.'..attr_name(rt)..';') end if rt:match('^Buffer$') or rt:match('^Window$') or rt:match('^Tabpage$') or rt:match('^Boolean$') then -- accept nonnegative integers for Booleans, Buffers, Windows and Tabpages @@ -368,6 +376,7 @@ output:write([[ #include "nvim/func_attr.h" #include "nvim/api/private/defs.h" +#include "nvim/api/private/helpers.h" #include "nvim/viml/executor/converter.h" ]]) include_headers(output, headers) @@ -382,27 +391,35 @@ local function process_function(fn) static int %s(lua_State *lstate) { Error err = {.set = false}; - ]], lua_c_function_name)) + if (lua_gettop(lstate) != %i) { + api_set_error(&err, Validation, "Expected %i argument%s"); + lua_pushstring(lstate, err.msg); + return lua_error(lstate); + } + ]], lua_c_function_name, #fn.parameters, #fn.parameters, + (#fn.parameters == 1) and '' or 's')) lua_c_functions[#lua_c_functions + 1] = { binding=lua_c_function_name, api=fn.name } - cparams = '' - for j, param in ipairs(fn.parameters) do + local cparams = '' + local free_code = {} + for j = #fn.parameters,1,-1 do + param = fn.parameters[j] cparam = string.format('arg%u', j) - if param[1]:match('^ArrayOf') then - param_type = 'Array' - else - param_type = param[1] - end + param_type = real_type(param[1]) + lc_param_type = param_type:lower() write_shifted_output(output, string.format([[ - %s %s = nlua_pop_%s(lstate, &err); + const %s %s = nlua_pop_%s(lstate, &err); + if (err.set) { + %s lua_pushstring(lstate, err.msg); return lua_error(lstate); } - ]], param[1], cparam, param_type)) - cparams = cparams .. cparam .. ', ' + ]], param[1], cparam, param_type, table.concat(free_code, '\n '))) + free_code[#free_code + 1] = ('api_free_%s(%s);'):format(lc_param_type, cparam) + cparams = cparam .. ', ' .. cparams end if fn.receives_channel_id then cparams = 'INTERNAL_CALL, ' .. cparams @@ -412,7 +429,7 @@ local function process_function(fn) else cparams = cparams:gsub(', $', '') end - local name = fn.impl_name or fn.name + free_at_exit_code = table.concat(free_code, '\n ') if fn.return_type ~= 'void' then if fn.return_type:match('^ArrayOf') then return_type = 'Array' @@ -420,23 +437,27 @@ local function process_function(fn) return_type = fn.return_type end write_shifted_output(output, string.format([[ - %s ret = %s(%s); + const %s ret = %s(%s); + %s if (err.set) { lua_pushstring(lstate, err.msg); return lua_error(lstate); } nlua_push_%s(lstate, ret); + api_free_%s(ret); return 1; - ]], fn.return_type, name, cparams, return_type)) + ]], fn.return_type, fn.name, cparams, free_at_exit_code, return_type, + return_type:lower())) else write_shifted_output(output, string.format([[ %s(%s); + %s if (err.set) { lua_pushstring(lstate, err.msg); return lua_error(lstate); } return 0; - ]], name, cparams)) + ]], fn.name, cparams, free_at_exit_code)) end write_shifted_output(output, [[ } @@ -457,11 +478,12 @@ void nlua_add_api_functions(lua_State *lstate) ]], #lua_c_functions)) for _, func in ipairs(lua_c_functions) do output:write(string.format([[ + lua_pushcfunction(lstate, &%s); - lua_setfield(lstate, -2, "%s"); - ]], func.binding, func.api)) + lua_setfield(lstate, -2, "%s");]], func.binding, func.api)) end output:write([[ + lua_setfield(lstate, -2, "api"); } ]]) diff --git a/src/nvim/api/private/helpers.c b/src/nvim/api/private/helpers.c index 7efa086af2..23d1540e2f 100644 --- a/src/nvim/api/private/helpers.c +++ b/src/nvim/api/private/helpers.c @@ -351,7 +351,7 @@ void set_option_to(void *to, int type, String name, Object value, Error *err) #define TYPVAL_ENCODE_CONV_UNSIGNED_NUMBER TYPVAL_ENCODE_CONV_NUMBER #define TYPVAL_ENCODE_CONV_FLOAT(tv, flt) \ - kv_push(edata->stack, FLOATING_OBJ((Float)(flt))) + kv_push(edata->stack, FLOAT_OBJ((Float)(flt))) #define TYPVAL_ENCODE_CONV_STRING(tv, str, len) \ do { \ diff --git a/src/nvim/api/private/helpers.h b/src/nvim/api/private/helpers.h index 9fe8c351cf..640e901fa1 100644 --- a/src/nvim/api/private/helpers.h +++ b/src/nvim/api/private/helpers.h @@ -27,7 +27,7 @@ .type = kObjectTypeInteger, \ .data.integer = i }) -#define FLOATING_OBJ(f) ((Object) { \ +#define FLOAT_OBJ(f) ((Object) { \ .type = kObjectTypeFloat, \ .data.floating = f }) diff --git a/src/nvim/api/vim.c b/src/nvim/api/vim.c index 5d862628cb..3fd1f57ace 100644 --- a/src/nvim/api/vim.c +++ b/src/nvim/api/vim.c @@ -139,7 +139,7 @@ String nvim_replace_termcodes(String str, Boolean from_part, Boolean do_lt, { if (str.size == 0) { // Empty string - return str; + return (String) { .data = NULL, .size = 0 }; } char *ptr = NULL; @@ -843,7 +843,7 @@ static void write_msg(String message, bool to_err) /// @return its argument. Object _vim_id(Object obj) { - return obj; + return copy_object(obj); } /// Returns array given as argument @@ -856,7 +856,7 @@ Object _vim_id(Object obj) /// @return its argument. Array _vim_id_array(Array arr) { - return arr; + return copy_object(ARRAY_OBJ(arr)).data.array; } /// Returns dictionary given as argument @@ -869,5 +869,18 @@ Array _vim_id_array(Array arr) /// @return its argument. Dictionary _vim_id_dictionary(Dictionary dct) { - return dct; + return copy_object(DICTIONARY_OBJ(dct)).data.dictionary; +} + +/// Returns floating-point value given as argument +/// +/// This API function is used for testing. One should not rely on its presence +/// in plugins. +/// +/// @param[in] flt Value to return. +/// +/// @return its argument. +Float _vim_id_float(Float flt) +{ + return flt; } diff --git a/src/nvim/msgpack_rpc/helpers.c b/src/nvim/msgpack_rpc/helpers.c index 5137b375f0..64a018f5c3 100644 --- a/src/nvim/msgpack_rpc/helpers.c +++ b/src/nvim/msgpack_rpc/helpers.c @@ -117,7 +117,7 @@ bool msgpack_rpc_to_object(const msgpack_object *const obj, Object *const arg) case MSGPACK_OBJECT_FLOAT: { STATIC_ASSERT(sizeof(Float) == sizeof(cur.mobj->via.f64), "Msgpack floating-point size does not match API integer"); - *cur.aobj = FLOATING_OBJ(cur.mobj->via.f64); + *cur.aobj = FLOAT_OBJ(cur.mobj->via.f64); break; } #define STR_CASE(type, attr, obj, dest, conv) \ diff --git a/src/nvim/viml/executor/converter.c b/src/nvim/viml/executor/converter.c index a6399500f2..a741d3a752 100644 --- a/src/nvim/viml/executor/converter.c +++ b/src/nvim/viml/executor/converter.c @@ -724,16 +724,15 @@ void nlua_push_Object(lua_State *lstate, const Object obj) String nlua_pop_String(lua_State *lstate, Error *err) FUNC_ATTR_NONNULL_ALL FUNC_ATTR_WARN_UNUSED_RESULT { - String ret; - - ret.data = (char *)lua_tolstring(lstate, -1, &(ret.size)); - - if (ret.data == NULL) { + if (lua_type(lstate, -1) != LUA_TSTRING) { lua_pop(lstate, 1); - set_api_error("Expected lua string", err); + api_set_error(err, Validation, "Expected lua string"); return (String) { .size = 0, .data = NULL }; } + String ret; + ret.data = (char *)lua_tolstring(lstate, -1, &(ret.size)); + assert(ret.data != NULL); ret.data = xmemdupz(ret.data, ret.size); lua_pop(lstate, 1); @@ -746,17 +745,19 @@ String nlua_pop_String(lua_State *lstate, Error *err) Integer nlua_pop_Integer(lua_State *lstate, Error *err) FUNC_ATTR_NONNULL_ALL FUNC_ATTR_WARN_UNUSED_RESULT { - Integer ret = 0; - - if (!lua_isnumber(lstate, -1)) { + if (lua_type(lstate, -1) != LUA_TNUMBER) { lua_pop(lstate, 1); - set_api_error("Expected lua integer", err); - return ret; + api_set_error(err, Validation, "Expected lua number"); + return 0; } - ret = (Integer)lua_tonumber(lstate, -1); + const lua_Number n = lua_tonumber(lstate, -1); lua_pop(lstate, 1); - - return ret; + if (n > (lua_Number)API_INTEGER_MAX || n < (lua_Number)API_INTEGER_MIN + || ((lua_Number)((Integer)n)) != n) { + api_set_error(err, Exception, "Number is not integral"); + return 0; + } + return (Integer)n; } /// Convert lua value to boolean @@ -765,7 +766,7 @@ Integer nlua_pop_Integer(lua_State *lstate, Error *err) Boolean nlua_pop_Boolean(lua_State *lstate, Error *err) FUNC_ATTR_NONNULL_ALL FUNC_ATTR_WARN_UNUSED_RESULT { - Boolean ret = lua_toboolean(lstate, -1); + const Boolean ret = lua_toboolean(lstate, -1); lua_pop(lstate, 1); return ret; } @@ -784,7 +785,7 @@ static inline LuaTableProps nlua_check_type(lua_State *const lstate, { if (lua_type(lstate, -1) != LUA_TTABLE) { if (err) { - set_api_error("Expected lua table", err); + api_set_error(err, Validation, "Expected lua table"); } return (LuaTableProps) { .type = kObjectTypeNil }; } @@ -797,7 +798,7 @@ static inline LuaTableProps nlua_check_type(lua_State *const lstate, if (table_props.type != type) { if (err) { - set_api_error("Unexpected type", err); + api_set_error(err, Validation, "Unexpected type"); } } @@ -1050,7 +1051,7 @@ Object nlua_pop_Object(lua_State *const lstate, Error *const err) const lua_Number n = lua_tonumber(lstate, -1); if (n > (lua_Number)API_INTEGER_MAX || n < (lua_Number)API_INTEGER_MIN || ((lua_Number)((Integer)n)) != n) { - *cur.obj = FLOATING_OBJ((Float)n); + *cur.obj = FLOAT_OBJ((Float)n); } else { *cur.obj = INTEGER_OBJ((Integer)n); } @@ -1094,7 +1095,7 @@ Object nlua_pop_Object(lua_State *const lstate, Error *const err) break; } case kObjectTypeFloat: { - *cur.obj = FLOATING_OBJ((Float)table_props.val); + *cur.obj = FLOAT_OBJ((Float)table_props.val); break; } case kObjectTypeNil: { diff --git a/test/functional/api/vim_spec.lua b/test/functional/api/vim_spec.lua index 3348368a36..24ed0afe67 100644 --- a/test/functional/api/vim_spec.lua +++ b/test/functional/api/vim_spec.lua @@ -219,6 +219,17 @@ describe('api', function() eq('\128\253\44', helpers.nvim('replace_termcodes', '<LeftMouse>', true, true, true)) end) + + it('does not crash when transforming an empty string', function() + -- Actually does not test anything, because current code will use NULL for + -- an empty string. + -- + -- Problem here is that if String argument has .data in allocated memory + -- then `return str` in vim_replace_termcodes body will make Neovim free + -- `str.data` twice: once when freeing arguments, then when freeing return + -- value. + eq('', meths.replace_termcodes('', true, true, true)) + end) end) describe('nvim_feedkeys', function() diff --git a/test/functional/lua_spec.lua b/test/functional/lua_spec.lua index 082efe4c0e..8ca47718aa 100644 --- a/test/functional/lua_spec.lua +++ b/test/functional/lua_spec.lua @@ -276,12 +276,53 @@ describe('luaeval() function', function() end) it('errors out correctly when working with API', function() - eq(0, exc_exec([[call luaeval("vim.api.id")]])) + -- Conversion errors + eq('Vim(call):E5108: Error while calling lua chunk for luaeval(): Cannot convert given lua type', + exc_exec([[call luaeval("vim.api._vim_id(vim.api._vim_id)")]])) + eq('Vim(call):E5108: Error while calling lua chunk for luaeval(): Cannot convert given lua table', + exc_exec([[call luaeval("vim.api._vim_id({1, foo=42})")]])) + eq('Vim(call):E5108: Error while calling lua chunk for luaeval(): Cannot convert given lua type', + exc_exec([[call luaeval("vim.api._vim_id({42, vim.api._vim_id})")]])) + -- Errors in number of arguments + eq('Vim(call):E5108: Error while calling lua chunk for luaeval(): Expected 1 argument', + exc_exec([[call luaeval("vim.api._vim_id()")]])) + eq('Vim(call):E5108: Error while calling lua chunk for luaeval(): Expected 1 argument', + exc_exec([[call luaeval("vim.api._vim_id(1, 2)")]])) + eq('Vim(call):E5108: Error while calling lua chunk for luaeval(): Expected 2 arguments', + exc_exec([[call luaeval("vim.api.vim_set_var(1, 2, 3)")]])) + -- Error in argument types + eq('Vim(call):E5108: Error while calling lua chunk for luaeval(): Expected lua string', + exc_exec([[call luaeval("vim.api.vim_set_var(1, 2)")]])) + + eq('Vim(call):E5108: Error while calling lua chunk for luaeval(): Expected lua number', + exc_exec([[call luaeval("vim.api.buffer_get_line(0, 'test')")]])) + eq('Vim(call):E5108: Error while calling lua chunk for luaeval(): Number is not integral', + exc_exec([[call luaeval("vim.api.buffer_get_line(0, 1.5)")]])) + + eq('Vim(call):E5108: Error while calling lua chunk for luaeval(): Expected lua table', + exc_exec([[call luaeval("vim.api._vim_id_float('test')")]])) + eq('Vim(call):E5108: Error while calling lua chunk for luaeval(): Unexpected type', + exc_exec([[call luaeval("vim.api._vim_id_float({[vim.type_idx]=vim.types.dictionary})")]])) + + eq('Vim(call):E5108: Error while calling lua chunk for luaeval(): Expected lua table', + exc_exec([[call luaeval("vim.api._vim_id_array(1)")]])) + eq('Vim(call):E5108: Error while calling lua chunk for luaeval(): Unexpected type', + exc_exec([[call luaeval("vim.api._vim_id_array({[vim.type_idx]=vim.types.dictionary})")]])) + + eq('Vim(call):E5108: Error while calling lua chunk for luaeval(): Expected lua table', + exc_exec([[call luaeval("vim.api._vim_id_dictionary(1)")]])) + eq('Vim(call):E5108: Error while calling lua chunk for luaeval(): Unexpected type', + exc_exec([[call luaeval("vim.api._vim_id_dictionary({[vim.type_idx]=vim.types.array})")]])) + -- TODO: check for errors with Tabpage argument + -- TODO: check for errors with Window argument + -- TODO: check for errors with Buffer argument + end) + + it('accepts any value as API Boolean', function() + eq('', funcs.luaeval('vim.api.vim_replace_termcodes("", vim, false, nil)')) + eq('', funcs.luaeval('vim.api.vim_replace_termcodes("", 0, 1.5, "test")')) + eq('', funcs.luaeval('vim.api.vim_replace_termcodes("", true, {}, {[vim.type_idx]=vim.types.array})')) end) -- TODO: check buffer/window/etc. - -- TODO: check what happens when it errors out on second list item - -- TODO: check what happens if API function receives wrong number of - -- arguments. - -- TODO: check what happens if API function receives wrong argument types. end) |