[LTP] [PATCH] sparse: Skip TCID and TST_TOTAL

[LTP] [PATCH] sparse: Skip TCID and TST_TOTAL

[Cyril Hrubis]

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

Re: [LTP] [PATCH] sparse: Skip TCID and TST_TOTAL

[Richard Palethorpe]

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

Re: [LTP] [PATCH] sparse: Skip TCID and TST_TOTAL

[Cyril Hrubis]

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

Re: [LTP] [PATCH] sparse: Skip TCID and TST_TOTAL

[Richard Palethorpe]

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

[LTP] [PATCH] check: Deprecated API symbols

[Richard Palethorpe via ltp]

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

Re: [LTP] [PATCH] check: Deprecated API symbols

[Cyril Hrubis]

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

Re: [LTP] [PATCH] check: Deprecated API symbols

[Richard Palethorpe]

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