From 45e5a18f3a8c44c5ecf54956d1a8248e1bf6f3e3 Mon Sep 17 00:00:00 2001 From: Thiago de Arruda Date: Fri, 16 May 2014 14:46:33 -0300 Subject: [PATCH] Enable -Wconversion for API files and fix errors --- src/nvim/CMakeLists.txt | 7 ++++ src/nvim/api/buffer.c | 88 +++++++++++++++++++++++++++------------ src/nvim/api/helpers.c | 40 ++++++++++++++---- src/nvim/api/vim.c | 37 +++++++++++----- src/nvim/api/vim.h | 3 +- src/nvim/api/window.c | 28 +++++++++++-- src/nvim/lib/khash.h | 10 ++--- src/nvim/os/msgpack_rpc.c | 21 +++------- 8 files changed, 165 insertions(+), 69 deletions(-) diff --git a/src/nvim/CMakeLists.txt b/src/nvim/CMakeLists.txt index a3d1f6ace9..221019ebe4 100644 --- a/src/nvim/CMakeLists.txt +++ b/src/nvim/CMakeLists.txt @@ -56,6 +56,13 @@ set(CONV_SRCS os/signal.c os/users.c os/wstream.c + os/msgpack_rpc.c + api/buffer.c + api/helpers.c + api/tabpage.c + api/window.c + api/vim.h + api/vim.c ) set_source_files_properties( diff --git a/src/nvim/api/buffer.c b/src/nvim/api/buffer.c index d3bec718aa..640b4d27a9 100644 --- a/src/nvim/api/buffer.c +++ b/src/nvim/api/buffer.c @@ -97,14 +97,32 @@ StringArray buffer_get_slice(Buffer buffer, return rv; } - rv.size = end - start; - rv.items = xmalloc(sizeof(String) * rv.size); + rv.size = (size_t)(end - start); + rv.items = xcalloc(sizeof(String), rv.size); - for (uint32_t i = 0; i < rv.size; i++) { - rv.items[i].data = xstrdup((char *)ml_get_buf(buf, start + i, false)); + for (size_t i = 0; i < rv.size; i++) { + int64_t lnum = start + (int64_t)i; + + if (lnum > LONG_MAX) { + set_api_error("Line index is too high", err); + goto end; + } + + rv.items[i].data = xstrdup((char *)ml_get_buf(buf, (linenr_T)lnum, false)); rv.items[i].size = strlen(rv.items[i].data); } +end: + if (err->set) { + for (size_t i = 0; i < rv.size; i++) { + if (rv.items[i].data != NULL) { + free(rv.items[i].data); + } + } + + free(rv.items); + } + return rv; } @@ -133,10 +151,9 @@ void buffer_set_slice(Buffer buffer, buf_T *save_curbuf = NULL; win_T *save_curwin = NULL; tabpage_T *save_curtab = NULL; - uint32_t new_len = replacement.size; - uint32_t old_len = end - start; - uint32_t i; - int32_t extra = 0; // lines added to text, can be negative + size_t new_len = replacement.size; + size_t old_len = (size_t)(end - start); + ssize_t extra = 0; // lines added to text, can be negative char **lines; if (new_len == 0) { @@ -146,7 +163,7 @@ void buffer_set_slice(Buffer buffer, lines = xcalloc(sizeof(char *), new_len); } - for (i = 0; i < new_len; i++) { + for (size_t i = 0; i < new_len; i++) { String l = replacement.items[i]; lines[i] = xstrndup(l.data, l.size); } @@ -154,30 +171,41 @@ void buffer_set_slice(Buffer buffer, try_start(); switch_to_win_for_buf(buf, &save_curwin, &save_curtab, &save_curbuf); - if (u_save(start - 1, end) == FAIL) { + if (u_save((linenr_T)(start - 1), (linenr_T)end) == FAIL) { set_api_error("Cannot save undo information", err); - goto cleanup; + goto end; } // If the size of the range is reducing (ie, new_len < old_len) we // need to delete some old_len. We do this at the start, by // repeatedly deleting line "start". - for (i = 0; new_len < old_len && i < old_len - new_len; i++) { - if (ml_delete(start, false) == FAIL) { + size_t to_delete = (new_len < old_len) ? (size_t)(old_len - new_len) : 0; + for (size_t i = 0; i < to_delete; i++) { + if (ml_delete((linenr_T)start, false) == FAIL) { set_api_error("Cannot delete line", err); - goto cleanup; + goto end; } } - extra -= i; + if ((ssize_t)to_delete > 0) { + extra -= (ssize_t)to_delete; + } // For as long as possible, replace the existing old_len with the // new old_len. This is a more efficient operation, as it requires // less memory allocation and freeing. - for (i = 0; i < old_len && i < new_len; i++) { - if (ml_replace(start + i, (char_u *)lines[i], false) == FAIL) { + size_t to_replace = old_len < new_len ? old_len : new_len; + for (size_t i = 0; i < to_replace; i++) { + int64_t lnum = start + (int64_t)i; + + if (lnum > LONG_MAX) { + set_api_error("Index value is too high", err); + goto end; + } + + if (ml_replace((linenr_T)lnum, (char_u *)lines[i], false) == FAIL) { set_api_error("Cannot replace line", err); - goto cleanup; + goto end; } // Mark lines that haven't been passed to the buffer as they need // to be freed later @@ -185,11 +213,19 @@ void buffer_set_slice(Buffer buffer, } // Now we may need to insert the remaining new old_len - while (i < new_len) { - if (ml_append(start + i - 1, (char_u *)lines[i], 0, false) == FAIL) { - set_api_error("Cannot insert line", err); - goto cleanup; + for (size_t i = to_replace; i < new_len; i++) { + int64_t lnum = start + (int64_t)i - 1; + + if (lnum > LONG_MAX) { + set_api_error("Index value is too high", err); + goto end; } + + if (ml_append((linenr_T)lnum, (char_u *)lines[i], 0, false) == FAIL) { + set_api_error("Cannot insert line", err); + goto end; + } + // Same as with replacing lines[i] = NULL; i++; @@ -201,16 +237,16 @@ void buffer_set_slice(Buffer buffer, // Only adjust marks if we managed to switch to a window that holds // the buffer, otherwise line numbers will be invalid. if (save_curbuf == NULL) { - mark_adjust(start, end - 1, MAXLNUM, extra); + mark_adjust((linenr_T)start, (linenr_T)(end - 1), MAXLNUM, extra); } - changed_lines(start, 0, end, extra); + changed_lines((linenr_T)start, 0, (linenr_T)end, extra); if (buf == curbuf) { - fix_cursor(start, end, extra); + fix_cursor((linenr_T)start, (linenr_T)end, extra); } -cleanup: +end: for (uint32_t i = 0; i < new_len; i++) { if (lines[i] != NULL) { free(lines[i]); diff --git a/src/nvim/api/helpers.c b/src/nvim/api/helpers.c index ea24e96a21..88ec12f3b5 100644 --- a/src/nvim/api/helpers.c +++ b/src/nvim/api/helpers.c @@ -123,7 +123,12 @@ Object dict_set_value(dict_T *dict, String key, Object value, Error *err) return rv; } - dictitem_T *di = dict_find(dict, (uint8_t *)key.data, key.size); + if (key.size > INT_MAX) { + set_api_error("Key length is too high", err); + return rv; + } + + dictitem_T *di = dict_find(dict, (uint8_t *)key.data, (int)key.size); if (value.type == kObjectTypeNil) { // Delete the key @@ -255,7 +260,12 @@ void set_option_to(void *to, int type, String name, Object value, Error *err) goto cleanup; } - int val = value.data.integer; + if (value.data.integer > INT_MAX || value.data.integer < INT_MIN) { + set_api_error("Option value outside range", err); + return; + } + + int val = (int)value.data.integer; set_option_value_for(key, val, NULL, opt_flags, type, to, err); } else { if (value.type != kObjectTypeString) { @@ -284,7 +294,12 @@ Object vim_to_object(typval_T *obj) buf_T *find_buffer(Buffer buffer, Error *err) { - buf_T *buf = buflist_findnr(buffer); + if (buffer > INT_MAX || buffer < INT_MIN) { + set_api_error("Invalid buffer id", err); + return NULL; + } + + buf_T *buf = buflist_findnr((int)buffer); if (buf == NULL) { set_api_error("Invalid buffer id", err); @@ -310,7 +325,12 @@ win_T * find_window(Window window, Error *err) tabpage_T * find_tab(Tabpage tabpage, Error *err) { - tabpage_T *rv = find_tabpage(tabpage); + if (tabpage > INT_MAX || tabpage < INT_MIN) { + set_api_error("Invalid tabpage id", err); + return NULL; + } + + tabpage_T *rv = find_tabpage((int)tabpage); if (!rv) { set_api_error("Invalid tabpage id", err); @@ -336,8 +356,13 @@ static bool object_to_vim(Object obj, typval_T *tv, Error *err) break; case kObjectTypeInteger: + if (obj.data.integer > INT_MAX || obj.data.integer < INT_MIN) { + set_api_error("Integer value outside range", err); + return false; + } + tv->v_type = VAR_NUMBER; - tv->vval.v_number = obj.data.integer; + tv->vval.v_number = (int)obj.data.integer; break; case kObjectTypeFloat: @@ -447,8 +472,9 @@ static Object vim_to_object_rec(typval_T *obj, khash_t(Lookup) *lookup) if (list != NULL) { rv.type = kObjectTypeArray; - rv.data.array.size = list->lv_len; - rv.data.array.items = xmalloc(list->lv_len * sizeof(Object)); + assert(list->lv_len >= 0); + rv.data.array.size = (size_t)list->lv_len; + rv.data.array.items = xmalloc(rv.data.array.size * sizeof(Object)); uint32_t i = 0; for (item = list->lv_first; item != NULL; item = item->li_next) { diff --git a/src/nvim/api/vim.c b/src/nvim/api/vim.c index bdd48336d1..505844e651 100644 --- a/src/nvim/api/vim.c +++ b/src/nvim/api/vim.c @@ -66,9 +66,14 @@ Object vim_eval(String str, Error *err) return rv; } -Integer vim_strwidth(String str) +Integer vim_strwidth(String str, Error *err) { - return mb_string2cells((char_u *)str.data, str.size); + if (str.size > INT_MAX) { + set_api_error("String length is too high", err); + return 0; + } + + return mb_string2cells((char_u *)str.data, (int)str.size); } StringArray vim_list_runtime_paths(void) @@ -99,10 +104,12 @@ StringArray vim_list_runtime_paths(void) while (*rtp != NUL) { rv.items[i].data = xmalloc(MAXPATHL); // Copy the path from 'runtimepath' to rv.items[i] - rv.items[i].size = copy_option_part(&rtp, - (char_u *)rv.items[i].data, - MAXPATHL, - ","); + int length = copy_option_part(&rtp, + (char_u *)rv.items[i].data, + MAXPATHL, + ","); + assert(length >= 0); + rv.items[i].size = (size_t)length; i++; } @@ -180,7 +187,7 @@ void vim_err_write(String str) Integer vim_get_buffer_count(void) { buf_T *b = firstbuf; - uint64_t n = 0; + Integer n = 0; while (b) { n++; @@ -197,8 +204,12 @@ Buffer vim_get_current_buffer(void) void vim_set_current_buffer(Buffer buffer, Error *err) { + if (!find_buffer(buffer, err)) { + return; + } + try_start(); - if (do_buffer(DOBUF_GOTO, DOBUF_FIRST, FORWARD, buffer, 0) == FAIL) { + if (do_buffer(DOBUF_GOTO, DOBUF_FIRST, FORWARD, (int)buffer, 0) == FAIL) { if (try_end(err)) { return; } @@ -216,7 +227,7 @@ Integer vim_get_window_count(void) { tabpage_T *tp; win_T *wp; - uint64_t rv = 0; + Integer rv = 0; FOR_ALL_TAB_WINDOWS(tp, wp) { rv++; @@ -268,7 +279,7 @@ void vim_set_current_window(Window window, Error *err) Integer vim_get_tabpage_count(void) { tabpage_T *tp = first_tabpage; - uint64_t rv = 0; + Integer rv = 0; while (tp != NULL) { tp = tp->tp_next; @@ -292,8 +303,12 @@ Tabpage vim_get_current_tabpage(void) void vim_set_current_tabpage(Tabpage tabpage, Error *err) { + if (!find_tab(tabpage, err)) { + return; + } + try_start(); - goto_tabpage(tabpage); + goto_tabpage((int)tabpage); try_end(err); } diff --git a/src/nvim/api/vim.h b/src/nvim/api/vim.h index 17a17c28c7..4ecc9fc2d0 100644 --- a/src/nvim/api/vim.h +++ b/src/nvim/api/vim.h @@ -30,8 +30,9 @@ Object vim_eval(String str, Error *err); /// one cell. /// /// @param str Some text +/// @param[out] err Details of an error that may have occurred /// @return The number of cells -Integer vim_strwidth(String str); +Integer vim_strwidth(String str, Error *err); /// Returns a list of paths contained in 'runtimepath' /// diff --git a/src/nvim/api/window.c b/src/nvim/api/window.c index b0d2838640..8e21034c40 100644 --- a/src/nvim/api/window.c +++ b/src/nvim/api/window.c @@ -48,8 +48,18 @@ void window_set_cursor(Window window, Position pos, Error *err) return; } - win->w_cursor.lnum = pos.row; - win->w_cursor.col = pos.col; + if (pos.row > LONG_MAX || pos.row < LONG_MIN) { + set_api_error("Row value outside range", err); + return; + } + + if (pos.col > INT_MAX || pos.col < INT_MIN) { + set_api_error("Column value outside range", err); + return; + } + + win->w_cursor.lnum = (linenr_T)pos.row; + win->w_cursor.col = (colnr_T)pos.col; win->w_cursor.coladd = 0; // When column is out of range silently correct it. check_cursor_col_win(win); @@ -75,10 +85,15 @@ void window_set_height(Window window, Integer height, Error *err) return; } + if (height > INT_MAX || height < INT_MIN) { + set_api_error("Height value outside range", err); + return; + } + win_T *savewin = curwin; curwin = win; try_start(); - win_setheight(height); + win_setheight((int)height); curwin = savewin; try_end(err); } @@ -102,10 +117,15 @@ void window_set_width(Window window, Integer width, Error *err) return; } + if (width > INT_MAX || width < INT_MIN) { + set_api_error("Width value outside range", err); + return; + } + win_T *savewin = curwin; curwin = win; try_start(); - win_setwidth(width); + win_setwidth((int)width); curwin = savewin; try_end(err); } diff --git a/src/nvim/lib/khash.h b/src/nvim/lib/khash.h index ada7da05b1..ce2862a1f1 100644 --- a/src/nvim/lib/khash.h +++ b/src/nvim/lib/khash.h @@ -158,10 +158,10 @@ typedef khint_t khiter_t; #define __ac_isempty(flag, i) ((flag[i>>4]>>((i&0xfU)<<1))&2) #define __ac_isdel(flag, i) ((flag[i>>4]>>((i&0xfU)<<1))&1) #define __ac_iseither(flag, i) ((flag[i>>4]>>((i&0xfU)<<1))&3) -#define __ac_set_isdel_false(flag, i) (flag[i>>4]&=~(1ul<<((i&0xfU)<<1))) -#define __ac_set_isempty_false(flag, i) (flag[i>>4]&=~(2ul<<((i&0xfU)<<1))) -#define __ac_set_isboth_false(flag, i) (flag[i>>4]&=~(3ul<<((i&0xfU)<<1))) -#define __ac_set_isdel_true(flag, i) (flag[i>>4]|=1ul<<((i&0xfU)<<1)) +#define __ac_set_isdel_false(flag, i) (flag[i>>4]&=~(khint_t)(1ul<<((i&0xfU)<<1))) +#define __ac_set_isempty_false(flag, i) (flag[i>>4]&=~(khint_t)(2ul<<((i&0xfU)<<1))) +#define __ac_set_isboth_false(flag, i) (flag[i>>4]&=~(khint_t)(3ul<<((i&0xfU)<<1))) +#define __ac_set_isdel_true(flag, i) (flag[i>>4]|=(khint_t)1ul<<((i&0xfU)<<1)) #define __ac_fsize(m) ((m) < 16? 1 : (m)>>4) @@ -400,7 +400,7 @@ static const double __ac_HASH_UPPER = 0.77; static kh_inline khint_t __ac_X31_hash_string(const char *s) { khint_t h = (khint_t)*s; - if (h) for (++s ; *s; ++s) h = (h << 5) - h + (khint_t)*s; + if (h) for (++s ; *s; ++s) h = (h << 5) - h + (uint8_t)*s; return h; } /*! @function diff --git a/src/nvim/os/msgpack_rpc.c b/src/nvim/os/msgpack_rpc.c index ec9969f8f1..45832070b1 100644 --- a/src/nvim/os/msgpack_rpc.c +++ b/src/nvim/os/msgpack_rpc.c @@ -36,7 +36,7 @@ void msgpack_rpc_call(msgpack_object *req, msgpack_packer *res) } // Set the response id, which is the same as the request - msgpack_pack_int(res, req->via.array.ptr[1].via.u64); + msgpack_pack_uint64(res, req->via.array.ptr[1].via.u64); if (req->via.array.ptr[0].type != MSGPACK_OBJECT_POSITIVE_INTEGER) { msgpack_rpc_error("Message type must be an integer", res); @@ -81,10 +81,9 @@ bool msgpack_rpc_to_boolean(msgpack_object *obj, Boolean *arg) bool msgpack_rpc_to_integer(msgpack_object *obj, Integer *arg) { - if (obj->type == MSGPACK_OBJECT_POSITIVE_INTEGER && obj->via.u64 <= INT64_MAX) { - *arg = obj->via.u64; + *arg = (int64_t)obj->via.u64; return true; } @@ -177,18 +176,10 @@ bool msgpack_rpc_to_stringarray(msgpack_object *obj, StringArray *arg) bool msgpack_rpc_to_position(msgpack_object *obj, Position *arg) { - // positions are represented by integer arrays of size 2 - if (obj->type != MSGPACK_OBJECT_ARRAY - || obj->via.array.size != 2 - || obj->via.array.ptr[0].type != MSGPACK_OBJECT_POSITIVE_INTEGER - || obj->via.array.ptr[1].type != MSGPACK_OBJECT_POSITIVE_INTEGER) { - return false; - } - - arg->row = obj->via.array.ptr[0].via.u64; - arg->col = obj->via.array.ptr[1].via.u64; - - return true; + return obj->type == MSGPACK_OBJECT_ARRAY + && obj->via.array.size == 2 + && msgpack_rpc_to_integer(obj->via.array.ptr, &arg->row) + && msgpack_rpc_to_integer(obj->via.array.ptr + 1, &arg->col); }