* [LTP] [PATCH] sparse: Skip TCID and TST_TOTAL
@ 2021-11-30 11:45 Cyril Hrubis
2021-11-30 13:56 ` Richard Palethorpe
2021-12-07 12:54 ` [LTP] [PATCH] check: Deprecated API symbols Richard Palethorpe via ltp
0 siblings, 2 replies; 7+ messages in thread
From: Cyril Hrubis @ 2021-11-30 11:45 UTC (permalink / raw)
To: ltp
Since these are part of the old library API.
Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
---
tools/sparse/sparse-ltp.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/tools/sparse/sparse-ltp.c b/tools/sparse/sparse-ltp.c
index b1677d336..1a3b4089a 100644
--- a/tools/sparse/sparse-ltp.c
+++ b/tools/sparse/sparse-ltp.c
@@ -98,6 +98,9 @@ static void check_symbol_visibility(const struct symbol *const sym)
if (!(mod & MOD_TOPLEVEL))
return;
+ if (!strcmp(name, "TCID") || !strcmp(name, "TST_TOTAL"))
+ return;
+
if (has_lib_prefix && (mod & MOD_STATIC) && !(mod & MOD_INLINE)) {
warning(sym->pos,
"LTP-003: Symbol '%s' has the LTP public library prefix, but is static (private).",
--
2.32.0
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [LTP] [PATCH] sparse: Skip TCID and TST_TOTAL
2021-11-30 11:45 [LTP] [PATCH] sparse: Skip TCID and TST_TOTAL Cyril Hrubis
@ 2021-11-30 13:56 ` Richard Palethorpe
2021-11-30 14:27 ` Cyril Hrubis
2021-12-07 12:54 ` [LTP] [PATCH] check: Deprecated API symbols Richard Palethorpe via ltp
1 sibling, 1 reply; 7+ messages in thread
From: Richard Palethorpe @ 2021-11-30 13:56 UTC (permalink / raw)
To: Cyril Hrubis; +Cc: ltp
Hello,
Cyril Hrubis <chrubis@suse.cz> writes:
> Since these are part of the old library API.
>
> Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
> ---
> tools/sparse/sparse-ltp.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/tools/sparse/sparse-ltp.c b/tools/sparse/sparse-ltp.c
> index b1677d336..1a3b4089a 100644
> --- a/tools/sparse/sparse-ltp.c
> +++ b/tools/sparse/sparse-ltp.c
> @@ -98,6 +98,9 @@ static void check_symbol_visibility(const struct symbol *const sym)
> if (!(mod & MOD_TOPLEVEL))
> return;
>
> + if (!strcmp(name, "TCID") || !strcmp(name, "TST_TOTAL"))
> + return;
Would it be better to print a warning that old library crap has been
detected?
My expectation is that 'make check' will produce a lot of noise if ran
on tests which haven't even been converted to the new library. Also it's
possible someone may forget to remove TCID etc.
> +
> if (has_lib_prefix && (mod & MOD_STATIC) && !(mod & MOD_INLINE)) {
> warning(sym->pos,
> "LTP-003: Symbol '%s' has the LTP public library prefix, but is static (private).",
> --
> 2.32.0
--
Thank you,
Richard.
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [LTP] [PATCH] sparse: Skip TCID and TST_TOTAL
2021-11-30 13:56 ` Richard Palethorpe
@ 2021-11-30 14:27 ` Cyril Hrubis
2021-12-01 7:21 ` Richard Palethorpe
0 siblings, 1 reply; 7+ messages in thread
From: Cyril Hrubis @ 2021-11-30 14:27 UTC (permalink / raw)
To: Richard Palethorpe; +Cc: ltp
Hi!
> > Since these are part of the old library API.
> >
> > Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
> > ---
> > tools/sparse/sparse-ltp.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/tools/sparse/sparse-ltp.c b/tools/sparse/sparse-ltp.c
> > index b1677d336..1a3b4089a 100644
> > --- a/tools/sparse/sparse-ltp.c
> > +++ b/tools/sparse/sparse-ltp.c
> > @@ -98,6 +98,9 @@ static void check_symbol_visibility(const struct symbol *const sym)
> > if (!(mod & MOD_TOPLEVEL))
> > return;
> >
> > + if (!strcmp(name, "TCID") || !strcmp(name, "TST_TOTAL"))
> > + return;
>
> Would it be better to print a warning that old library crap has been
> detected?
I guess so.
> My expectation is that 'make check' will produce a lot of noise if ran
> on tests which haven't even been converted to the new library. Also it's
> possible someone may forget to remove TCID etc.
Well we may as well print the warnings only if new library test is
detected. But I guess that is not an easy task since we would have to
look at the include directives even before we attempt to parse the
file..
--
Cyril Hrubis
chrubis@suse.cz
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [LTP] [PATCH] sparse: Skip TCID and TST_TOTAL
2021-11-30 14:27 ` Cyril Hrubis
@ 2021-12-01 7:21 ` Richard Palethorpe
0 siblings, 0 replies; 7+ messages in thread
From: Richard Palethorpe @ 2021-12-01 7:21 UTC (permalink / raw)
To: Cyril Hrubis; +Cc: ltp
Hello,
Cyril Hrubis <chrubis@suse.cz> writes:
> Hi!
>> > Since these are part of the old library API.
>> >
>> > Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
>> > ---
>> > tools/sparse/sparse-ltp.c | 3 +++
>> > 1 file changed, 3 insertions(+)
>> >
>> > diff --git a/tools/sparse/sparse-ltp.c b/tools/sparse/sparse-ltp.c
>> > index b1677d336..1a3b4089a 100644
>> > --- a/tools/sparse/sparse-ltp.c
>> > +++ b/tools/sparse/sparse-ltp.c
>> > @@ -98,6 +98,9 @@ static void check_symbol_visibility(const struct symbol *const sym)
>> > if (!(mod & MOD_TOPLEVEL))
>> > return;
>> >
>> > + if (!strcmp(name, "TCID") || !strcmp(name, "TST_TOTAL"))
>> > + return;
>>
>> Would it be better to print a warning that old library crap has been
>> detected?
>
> I guess so.
>
>> My expectation is that 'make check' will produce a lot of noise if ran
>> on tests which haven't even been converted to the new library. Also it's
>> possible someone may forget to remove TCID etc.
>
> Well we may as well print the warnings only if new library test is
> detected. But I guess that is not an easy task since we would have to
> look at the include directives even before we attempt to parse the
> file..
Some checks may require detecting the type of translation unit in an
initial pass. At which point we will probably get this for free.
--
Thank you,
Richard.
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 7+ messages in thread
* [LTP] [PATCH] check: Deprecated API symbols
2021-11-30 11:45 [LTP] [PATCH] sparse: Skip TCID and TST_TOTAL Cyril Hrubis
2021-11-30 13:56 ` Richard Palethorpe
@ 2021-12-07 12:54 ` Richard Palethorpe via ltp
2021-12-07 12:59 ` Cyril Hrubis
1 sibling, 1 reply; 7+ messages in thread
From: Richard Palethorpe via ltp @ 2021-12-07 12:54 UTC (permalink / raw)
To: ltp; +Cc: Richard Palethorpe
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(+)
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);
}
--
2.34.0
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [LTP] [PATCH] check: Deprecated API symbols
2021-12-07 12:54 ` [LTP] [PATCH] check: Deprecated API symbols Richard Palethorpe via ltp
@ 2021-12-07 12:59 ` Cyril Hrubis
2021-12-07 14:09 ` Richard Palethorpe
0 siblings, 1 reply; 7+ messages in thread
From: Cyril Hrubis @ 2021-12-07 12:59 UTC (permalink / raw)
To: Richard Palethorpe; +Cc: ltp
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>
--
Cyril Hrubis
chrubis@suse.cz
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [LTP] [PATCH] check: Deprecated API symbols
2021-12-07 12:59 ` Cyril Hrubis
@ 2021-12-07 14:09 ` Richard Palethorpe
0 siblings, 0 replies; 7+ messages in thread
From: Richard Palethorpe @ 2021-12-07 14:09 UTC (permalink / raw)
To: Cyril Hrubis; +Cc: ltp
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!
--
Thank you,
Richard.
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-12-07 14:12 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-30 11:45 [LTP] [PATCH] sparse: Skip TCID and TST_TOTAL Cyril Hrubis
2021-11-30 13:56 ` Richard Palethorpe
2021-11-30 14:27 ` Cyril Hrubis
2021-12-01 7:21 ` Richard Palethorpe
2021-12-07 12:54 ` [LTP] [PATCH] check: Deprecated API symbols Richard Palethorpe via ltp
2021-12-07 12:59 ` Cyril Hrubis
2021-12-07 14:09 ` Richard Palethorpe
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.