On Mon, Nov 04, 2019 at 10:46:44PM +0100, Luc Van Oostenryck wrote: > On Fri, Nov 01, 2019 at 04:36:51PM +0000, Ben Dooks wrote: > > I've put the latest code up at: > > > > https://github.com/bjdooks-ct/sparse bjdooks/printf20 > > > > I think it has all the issues dealt with. > > > > I can't currently post or do a final test as away from work laptop. > > Thank you. > > I've a few more remarks about formatting or naming and > also some simplifications I would like you make. This one is a bit more complex. IIRC, you introduced get_printf_fmt() and called before the evaluation of the arguments because this evaluation inhibited the check that the format was indeed a string literal. In fact it doesn't. This function is thus not needed and everything can be done in the same function. Uou'll need a few more changes at the beginning of evaluate_format_printf() so that the warning is again issued for non-literal string format. >From bc1c1dd9851188f2de25cdbd2ca4904f6009a0c3 Mon Sep 17 00:00:00 2001 From: Luc Van Oostenryck <luc.vanoostenryck@xxxxxxxxx> Date: Mon, 4 Nov 2019 18:04:38 +0100 Subject: [PATCH 4/7] get rid of get_printf_fmt() --- evaluate.c | 51 +++++++++++++++------------------------------------ 1 file changed, 15 insertions(+), 36 deletions(-) diff --git a/evaluate.c b/evaluate.c index c415773f4..76b37e045 100644 --- a/evaluate.c +++ b/evaluate.c @@ -2706,44 +2706,33 @@ static int parse_format_printf(const char **fmtstring, return 1; } -static const char *get_printf_fmt(struct symbol *fn, struct expression_list *head) -{ - struct expression *expr; - - expr = get_nth_expression(head, fn->ctype.format.index-1); - if (!expr) - return NULL; - if (!evaluate_expression(expr)) - return NULL; - if (expr->type == EXPR_PREOP && expr->op == '*') - expr = expr->unop; - if (expr->type == EXPR_SYMBOL) { - struct symbol *sym = expr->symbol; - if (sym && sym->initializer) - expr = sym->initializer; - } - if (expr->type == EXPR_STRING) - return expr->string->data; - - return NULL; -} - /* * attempt to run through a printf format string and work out the types * it specifies. The format is parsed from the __attribute__(format()) * in the parser code which stores the positions of the message and arg * start in the ctype. */ -static void evaluate_format_printf(const char *fmt_string, struct symbol *fn, - struct expression_list *head) +static void evaluate_format_printf(struct symbol *fn, struct expression_list *head) { struct format_state state = { }; struct expression *expr; + struct expression *init; + const char *fmt_string; + + if (!fn->ctype.format.index) + return; expr = get_nth_expression(head, fn->ctype.format.index-1); if (!expr) return; + if (expr->type != EXPR_SYMBOL || expr->symbol->ident) + return; // not a literal + init = expr->symbol->initializer; + if (!init || init->type != EXPR_STRING) + return; // not a string + fmt_string = init->string->data; + state.expr = expr; state.first = fn->ctype.format.first; state.arg_index = fn->ctype.format.first; @@ -2775,19 +2764,9 @@ static int evaluate_arguments(struct symbol *fn, struct expression_list *head) { struct expression *expr; struct symbol_list *argument_types = fn->arguments; - const char *fmt_string = NULL; struct symbol *argtype; int i = 1; - /* - * Do the va-arg format parsing here. This is done as the argument - * info may get lost or changed later on in the evaluation loop by - * calls to degenerate() - */ - - if (Wformat && fn->ctype.format.index) - fmt_string = get_printf_fmt(fn, head); - PREPARE_PTR_LIST(argument_types, argtype); FOR_EACH_PTR (head, expr) { struct expression **p = THIS_ADDRESS(expr); @@ -2825,8 +2804,8 @@ static int evaluate_arguments(struct symbol *fn, struct expression_list *head) } END_FOR_EACH_PTR(expr); FINISH_PTR_LIST(argtype); - if (Wformat && fn->ctype.format.index) - evaluate_format_printf(fmt_string, fn, head); + if (Wformat) + evaluate_format_printf(fn, head); return 1; } -- 2.23.0
