| Message ID | 20211207125420.32538-1-rpalethorpe@suse.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | check: Deprecated API symbols | expand |
Hi! > The old API represents a big source of complication. It invalidates a > lot of assumptions we can make when writing checks specifically for > the new API. > > Cyril proposed ignoring these symbols altogether in a previous > patch. This is a counter proposal to print a warning, but then abandon > checking the symbol any further. > > The reasoning is that ignoring them altogether might hide errors. Also > the existence of the old API may be a surprise to new developers. This > change will alert them that a test needs updating and that it is not > to be used as a reference. > > Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com> Sounds good to me: Reviewed-by: Cyril Hrubis <chrubis@suse.cz>
Hello, Cyril Hrubis <chrubis@suse.cz> writes: > Hi! >> The old API represents a big source of complication. It invalidates a >> lot of assumptions we can make when writing checks specifically for >> the new API. >> >> Cyril proposed ignoring these symbols altogether in a previous >> patch. This is a counter proposal to print a warning, but then abandon >> checking the symbol any further. >> >> The reasoning is that ignoring them altogether might hide errors. Also >> the existence of the old API may be a surprise to new developers. This >> change will alert them that a test needs updating and that it is not >> to be used as a reference. >> >> Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com> > > Sounds good to me: > > Reviewed-by: Cyril Hrubis <chrubis@suse.cz> Thanks, pushed!
diff --git a/tools/sparse/sparse-ltp.c b/tools/sparse/sparse-ltp.c index 3a38229f1..513033518 100644 --- a/tools/sparse/sparse-ltp.c +++ b/tools/sparse/sparse-ltp.c @@ -81,6 +81,32 @@ static void do_entrypoint_checks(struct entrypoint *ep) do_basicblock_checks(bb); } END_FOR_EACH_PTR(bb); } +/* The old API can not comply with the rules. So when we see one of + * these symbols we know that it will result in further + * warnings. Probably these will suggest inappropriate things. Usually + * these symbols should be removed and the new API used + * instead. Otherwise they can be ignored until all tests have been + * converted to the new API. + */ +static bool check_symbol_deprecated(const struct symbol *const sym) +{ + static struct ident *TCID_id, *TST_TOTAL_id; + const struct ident *id = sym->ident; + + if (!TCID_id) { + TCID_id = built_in_ident("TCID"); + TST_TOTAL_id = built_in_ident("TST_TOTAL"); + } + + if (id != TCID_id && id != TST_TOTAL_id) + return false; + + warning(sym->pos, + "Ignoring deprecated API symbol: '%s'. Should this code be converted to the new API?", + show_ident(id)); + + return true; +} /* Check for LTP-003 and LTP-004 * @@ -212,6 +238,9 @@ static void check_test_struct(const struct symbol *const sym) /* AST level checks */ static void do_symbol_checks(struct symbol *sym) { + if (check_symbol_deprecated(sym)) + return; + check_symbol_visibility(sym); check_test_struct(sym); }
The old API represents a big source of complication. It invalidates a lot of assumptions we can make when writing checks specifically for the new API. Cyril proposed ignoring these symbols altogether in a previous patch. This is a counter proposal to print a warning, but then abandon checking the symbol any further. The reasoning is that ignoring them altogether might hide errors. Also the existence of the old API may be a surprise to new developers. This change will alert them that a test needs updating and that it is not to be used as a reference. Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com> --- tools/sparse/sparse-ltp.c | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+)