From c8878088e0c4c3717991568b95cee44ea12e641d Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Sat, 18 Oct 2008 10:36:09 +0300 Subject: [PATCH] Fix memory leak. Add more comments. Signed-off-by: Dan Carpenter --- check_template.c | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/check_template.c b/check_template.c index 2c5cbfe8..0db28678 100644 --- a/check_template.c +++ b/check_template.c @@ -8,9 +8,6 @@ */ /* - * This is just a template check. It's designed for teaching - * only and doesn't even work. - * * First of all, it's best if you lower your expectations from finding * errors to just finding suspicious code. There tends to be a lot * of false positives so having low expectations helps. @@ -18,13 +15,24 @@ * For this test let's look for functions that return a negative value * with a semaphore held. * + * This is just a template check. It's designed for teaching + * only and is deliberately less useful than it could be. + * * This test could be a lot better if it handled the stuff like this: * ret = -ENOMEM; * return ret; * The correct way to handle that is to let smatch_extra store the * value of ret. Then to use a *future* version of smatch that has - * the get_possible_states() function. The possible states will - * be saved in merge_slist(). + * the get_possible_states(name, SMATCH_EXTRA, sym) function. The + * possible states will be saved in merge_slist() in that future version. + * + * Another short coming is that it assumes a function isn't supposed + * to return negative with a lock held. Perhaps the function was + * called with the lock held. A more complicated script could check that. + * + * Also it would be more effective to check for other types of locks + * especially spinlocks. + * */ #include "parse.h" @@ -57,24 +65,21 @@ static struct smatch_state *merge_func(const char *name, struct symbol *sym, static void match_call(struct expression *expr) { char *fn_name; - int down = 0; struct expression *sem_expr; char *sem_name; fn_name = get_variable_from_expr(expr->fn, NULL); if (!fn_name || (strcmp(fn_name, "down") && strcmp(fn_name, "up"))) return; - if (!strcmp(fn_name, "down")) - down = 1; - sem_expr = get_argument_from_call_expr(expr->args, 0); sem_name = get_variable_from_expr(sem_expr, NULL); - if (down) { + if (!strcmp(fn_name, "down")) { set_state(sem_name, my_id, NULL, &lock); } else { set_state(sem_name, my_id, NULL, &unlock); } + free_string(fn_name); } static void match_return(struct statement *stmt) @@ -86,7 +91,7 @@ static void match_return(struct statement *stmt) ret_val = get_value(stmt->ret_value); if (ret_val == UNDEFINED || ret_val >= 0) return; - + slist = get_all_states(my_id); FOR_EACH_PTR(slist, tmp) { -- 2.11.4.GIT