From d6472f459bc531bf6e24c3bcc1916dba16ba1f10 Mon Sep 17 00:00:00 2001 From: Eliseo Martínez Date: Sat, 8 Nov 2014 21:14:33 +0100 Subject: Fix warnings: ex_getln.c: init_history(): Double free: FP. Problem : Double free @ 4249. Diagnostic : False positive. Rationale : Codepath leading to error contains two consecutive iterations in which `if (--j < 0)` is true. That executes `free` two consecutive times with the same value (hislen - 1) for j, with leads to double free. Now, that can only happen with j == 0 && hislen == 1. And that would imply j == hisidx[type] too, which would take the following break. So, the error codepath cannot really happen, but the compiler cannot deduce the last implication. Resolution : We have two possible solutions for this: 1.- Comparing value of j before and after updating it, and breaking out of iteration if equal. That changes nothing in functionality, but teaches the compiler his proposed error codepath is impossible. 2.- Nullify pointer after freeing. This way, the compiler still thinks his error codepath is possible, but it's not an error anymore, as free(NULL) is a no-op. We opt for solution 2, as solution 1 requires adding logic that adds nothing (and having to explain that clearly in aside comments) just for the purpose of silencing warning. On the other hand, solution 2 improves the code, adding something considered good practice in any case, and therefore doesn't require further explanation. --- src/nvim/ex_getln.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/nvim/ex_getln.c b/src/nvim/ex_getln.c index b8d8837092..5feff4d456 100644 --- a/src/nvim/ex_getln.c +++ b/src/nvim/ex_getln.c @@ -4245,8 +4245,10 @@ void init_history(void) for (i = newlen - 1;; --i) { if (i >= 0) /* copy newest entries */ temp[i] = history[type][j]; - else /* remove older entries */ + else { /* remove older entries */ free(history[type][j].hisstr); + history[type][j].hisstr = NULL; + } if (--j < 0) j = hislen - 1; if (j == hisidx[type]) -- cgit