On Wed, Sep 30, 2020 at 09:35:50AM +0100, Ben Dooks wrote: > I've done a rebase to v0.6.2 and put the result up at: > > https://gitlab.com/CodethinkLabs/sparse bjdooks/printf-new3 Great. It just needs a few very mnor changes: %% diff --git a/evaluate.h b/evaluate.h %% index a16e97036b2a..bb8ec480691c 100644 %% --- a/evaluate.h %% +++ b/evaluate.h %% @@ -26,10 +26,14 @@ struct symbol *evaluate_statement(struct statement *stmt); %% // @list: the list of the symbol to be evaluated %% void evaluate_symbol_list(struct symbol_list *list); %% %% -/// This is the marker for autodoc, so it needs to stay. %% // evaluate the arguments of a function %% +// @fn: the symbol of the prototype %% // @argtypes: the list of the types in the prototype %% // @args: the list of the effective arguments %% -int evaluate_arguments(struct symbol_list *argtypes, struct expression_list *args); %% +int evaluate_arguments(struct symbol *fn, struct symbol_list *argtypes, struct expression_list *args); %% %% +// check if assignment types are compatible %% +// todo I've removed the 'todo' %% diff --git a/parse.c b/parse.c %% index 31ecef0f554d..a7ab5fd6e531 100644 %% --- a/parse.c %% +++ b/parse.c %% @@ -377,6 +383,10 @@ static struct symbol_op attr_force_op = { %% .attribute = attribute_force, %% }; %% %% +static struct symbol_op attr_format = { To stay coherent with existent naming, I've rename this into 'attr_format_op' %% @@ -515,6 +535,7 @@ static struct init_keyword { %% N("_Float64", &spec_op, .type = &float64_ctype), %% N("_Float64x", &spec_op, .type = &float64x_ctype), %% N("_Float128", &spec_op, .type = &float128_ctype), %% + Removd unneeded line. %% @@ -551,6 +572,9 @@ static struct init_keyword { %% D("pure", &attr_fun_op, .mods = MOD_PURE), %% A("const", &attr_fun_op, .mods = MOD_PURE), %% D("gnu_inline", &attr_fun_op, .mods = MOD_GNU_INLINE), %% + N("format", &attr_format), %% + N("printf", &attr_printf_op), %% + N("scanf", &attr_scanf_op), I've changed the 'N' into 'D' since the underscore versions are legit and often favored. %% diff --git a/sparse.1 b/sparse.1 %% index 48dab7a9a5c1..e46aafdb3e5e 100644 %% --- a/sparse.1 %% +++ b/sparse.1 %% @@ -275,6 +275,15 @@ trouble. %% Sparse does not issue these warnings by default. %% . %% .TP %% +.B \-Wformat %% +Warn about parameter mismatch to any variadic function which specifies %% +where the format string is specified with the %% +.BI __attribute__((format( type, message, va_start ))) %% +attribute. %% + %% +Sparse does not issue these warnings by default. To turn them on, use %% +\fB\-Wno\-format\fR %% +.TP I've moved this one position above, between '-Wexternal-...' and -Winit-.. to keep the alphabetical order, removed some unneeded whitespace and added a final dot. %% diff --git a/symbol.h b/symbol.h %% index a3ed95678ee5..47e26816430c 100644 %% --- a/symbol.h %% +++ b/symbol.h %% @@ -84,6 +84,7 @@ enum keyword { %% KW_STATIC = 1 << 7, %% // KW UNUSED = 1 << 8, %% KW_EXACT = 1 << 9, %% + KW_FORMAT = 1 << 10, %% }; I've just reused the UNUSED slot. I've also exchanged patch 3 & 4 because the 3rd needed the definition of Wformat only added in patch 4 and fixed 2 or 3 typos in the commit messages. I've pushed these changes at gitlab.com/lucvoo/sparse-dev format-check I've left for now the changelog parts in the commit messages but I would prefer to not have them in the final version. > Should I put this through a new round of review? I'm fine with the changes (moving the check to a verify-format.c + some changes related to excessively long lines). I've one last request, the email address given as author is not the same as the one used in the Signed-off-by and the copyright notice. So, is it possible to respin the series with: * the small changes I've made here above * removing the changelogs from the commit messages * using the same email address for the author and the SoB? Thanks, -- Luc
