[LTP] [PATCH 0/3] tools/sparse: Add static check

[LTP] [PATCH 0/3] tools/sparse: Add static check

[Richard Palethorpe via ltp]

Hello,

This teaches 'make check' to flag missing static keywords. Note that I
am not sure how this avoids flagging symbols only declared in header
files. However I haven't noticed any false positives and I think false
negatives will only occur when the author uses function prototypes to
reference private symbols before their definition.

The statx patch is just included to show one issue it caught.

Richard Palethorpe (3):
  tools/sparse: Add static check
  doc: Add LTP-003 and LTP-004 static and tst API prefix rules
  statx: Add missing static keyword to tcase variable

 doc/library-api-writing-guidelines.txt    | 10 +++++
 doc/rules.tsv                             |  2 +
 doc/test-writing-guidelines.txt           |  8 ++++
 testcases/kernel/syscalls/statx/statx01.c |  2 +-
 testcases/kernel/syscalls/statx/statx02.c |  2 +-
 testcases/kernel/syscalls/statx/statx04.c |  2 +-
 testcases/kernel/syscalls/statx/statx05.c |  2 +-
 testcases/kernel/syscalls/statx/statx07.c |  2 +-
 tools/sparse/sparse-ltp.c                 | 53 +++++++++++++++++++++++
 9 files changed, 78 insertions(+), 5 deletions(-)

-- 
2.33.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 1/3] tools/sparse: Add static check

[Richard Palethorpe via ltp]

This was adapted from Sparse's inbuilt check_duplicates (-Wdecl). The
original check appears to print a warning whenever a symbol is
non-static, but has no prototype. It appears to work because library
symbols are usually declared first in a header file and then again
with their definition in a source file.

The LTP version also checks for the various library prefixes, but
should otherwise be the same.

Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
---
 tools/sparse/sparse-ltp.c | 53 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 53 insertions(+)

diff --git a/tools/sparse/sparse-ltp.c b/tools/sparse/sparse-ltp.c
index 45874e8eb..73725d191 100644
--- a/tools/sparse/sparse-ltp.c
+++ b/tools/sparse/sparse-ltp.c
@@ -82,6 +82,57 @@ static void do_entrypoint_checks(struct entrypoint *ep)
 	} END_FOR_EACH_PTR(bb);
 }
 
+/* Check for LTP-003 and LTP-004
+ *
+ * Try to find cases where the static keyword was forgotten.
+ */
+static void check_symbol_visibility(struct symbol *sym)
+{
+	struct symbol *next = sym;
+	unsigned long mod = sym->ctype.modifiers;
+	const char *name = show_ident(sym->ident);
+	int has_lib_prefix = !strncmp("tst_", name, 4) ||
+		!strncmp("TST_", name, 4) ||
+		!strncmp("ltp_", name, 4) ||
+		!strncmp("safe_", name, 5);
+
+	if (!(mod & MOD_TOPLEVEL))
+		return;
+
+	if (has_lib_prefix && (mod & MOD_STATIC)) {
+		warning(sym->pos,
+			"LTP-003: Symbol '%s' has the LTP public library prefix, but is static (private).",
+			name);
+		return;
+	}
+
+	if ((mod & MOD_STATIC))
+		return;
+
+	if (tu_kind == LTP_LIB && !has_lib_prefix) {
+		warning(sym->pos,
+			"LTP-003: Symbol '%s' is a public library function, but is missing the 'tst_' prefix",
+			name);
+		return;
+	}
+
+	if (next->same_symbol)
+		return;
+
+	if (sym->ident == &main_ident)
+		return;
+
+	warning(sym->pos,
+		"Symbol '%s' has no prototype or library ('tst_') prefix. Should it be static?",
+		name);
+}
+
+/* AST level checks */
+static void do_symbol_checks(struct symbol *sym)
+{
+	check_symbol_visibility(sym);
+}
+
 /* Compile the AST into a graph of basicblocks */
 static void process_symbols(struct symbol_list *list)
 {
@@ -90,6 +141,8 @@ static void process_symbols(struct symbol_list *list)
 	FOR_EACH_PTR(list, sym) {
 		struct entrypoint *ep;
 
+		do_symbol_checks(sym);
+
 		expand_symbol(sym);
 		ep = linearize_symbol(sym);
 		if (!ep || !ep->entry)
-- 
2.33.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 2/3] doc: Add LTP-003 and LTP-004 static and tst API prefix rules

[Richard Palethorpe via ltp]

Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
---
 doc/library-api-writing-guidelines.txt | 10 ++++++++++
 doc/rules.tsv                          |  2 ++
 doc/test-writing-guidelines.txt        |  8 ++++++++
 3 files changed, 20 insertions(+)

diff --git a/doc/library-api-writing-guidelines.txt b/doc/library-api-writing-guidelines.txt
index 2819d4177..c82053681 100644
--- a/doc/library-api-writing-guidelines.txt
+++ b/doc/library-api-writing-guidelines.txt
@@ -39,6 +39,16 @@ The macros which are clearly intended to update these variables. That
 is +TEST+ and those in 'tst_test_macros.h'. Are of course allowed to
 update these variables.
 
+2.3 LTP-003: Externally visible library symbols have the tst_ prefix
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Functions, types and variables in the public test API should have the
+tst_ prefix. With some exceptions for symbols already prefixed with
+safe_ or ltp_.
+
+Static (private) symbols should not have the prefix.
+
+
 3. Shell API
 ------------
 
diff --git a/doc/rules.tsv b/doc/rules.tsv
index d4081ce0f..2dfeca9f9 100644
--- a/doc/rules.tsv
+++ b/doc/rules.tsv
@@ -1,3 +1,5 @@
 ID	DESCRIPTION
 LTP-001	Library source files have tst_ prefix
 LTP-002	TST_RET and TST_ERR are never modified by test library functions
+LTP-003 Externally visible library symbols have the tst_ prefix
+LTP-004 Test executable symbols are marked static
diff --git a/doc/test-writing-guidelines.txt b/doc/test-writing-guidelines.txt
index b87446d1b..98fdb4d8d 100644
--- a/doc/test-writing-guidelines.txt
+++ b/doc/test-writing-guidelines.txt
@@ -133,6 +133,14 @@ script from kernel git tree.
 NOTE: If `make check` does not report any problems, the code still may be wrong
       as all tools used for checking only look for common mistakes.
 
+2.1.1 LTP-004: Test executable symbols are marked static
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+Test executables should not export symbols unecessarily. This means
+that all top-level variables and functions should be marked with the
+static keyword. The only visible symbols should be those included from
+shared object files.
+
 2.2 Shell coding style
 ^^^^^^^^^^^^^^^^^^^^^^
 
-- 
2.33.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 3/3] statx: Add missing static keyword to tcase variable

[Richard Palethorpe via ltp]

This shows how easy it is to miss the static keyword.

Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
---
 testcases/kernel/syscalls/statx/statx01.c | 2 +-
 testcases/kernel/syscalls/statx/statx02.c | 2 +-
 testcases/kernel/syscalls/statx/statx04.c | 2 +-
 testcases/kernel/syscalls/statx/statx05.c | 2 +-
 testcases/kernel/syscalls/statx/statx07.c | 2 +-
 5 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/testcases/kernel/syscalls/statx/statx01.c b/testcases/kernel/syscalls/statx/statx01.c
index 2358dd7bc..11e188e8f 100644
--- a/testcases/kernel/syscalls/statx/statx01.c
+++ b/testcases/kernel/syscalls/statx/statx01.c
@@ -131,7 +131,7 @@ static void test_device_file(void)
 }
 
 
-struct tcase {
+static struct tcase {
 	void (*tfunc)(void);
 } tcases[] = {
 	{&test_normal_file},
diff --git a/testcases/kernel/syscalls/statx/statx02.c b/testcases/kernel/syscalls/statx/statx02.c
index 08ea940cb..15f5562b2 100644
--- a/testcases/kernel/syscalls/statx/statx02.c
+++ b/testcases/kernel/syscalls/statx/statx02.c
@@ -90,7 +90,7 @@ static void test_sym_link(void)
 			"Statx symlink flag failed to work as expected");
 }
 
-struct tcase {
+static struct tcase {
 	void (*tfunc)(void);
 } tcases[] = {
 	{&test_empty_path},
diff --git a/testcases/kernel/syscalls/statx/statx04.c b/testcases/kernel/syscalls/statx/statx04.c
index 180c61bf9..f66b04f70 100644
--- a/testcases/kernel/syscalls/statx/statx04.c
+++ b/testcases/kernel/syscalls/statx/statx04.c
@@ -133,7 +133,7 @@ static void test_unflagged(void)
 		tst_res(TFAIL, "STATX_ATTR_NODUMP flag is set");
 }
 
-struct test_cases {
+static struct test_cases {
 	void (*tfunc)(void);
 } tcases[] = {
 	{&test_flagged},
diff --git a/testcases/kernel/syscalls/statx/statx05.c b/testcases/kernel/syscalls/statx/statx05.c
index 81a5bcbf2..47a1f8ad1 100644
--- a/testcases/kernel/syscalls/statx/statx05.c
+++ b/testcases/kernel/syscalls/statx/statx05.c
@@ -74,7 +74,7 @@ static void test_unflagged(void)
 		tst_res(TFAIL, "STATX_ATTR_ENCRYPTED flag is set");
 }
 
-struct test_cases {
+static struct test_cases {
 	void (*tfunc)(void);
 } tcases[] = {
 	{&test_flagged},
diff --git a/testcases/kernel/syscalls/statx/statx07.c b/testcases/kernel/syscalls/statx/statx07.c
index ec1cdd190..9aa160d31 100644
--- a/testcases/kernel/syscalls/statx/statx07.c
+++ b/testcases/kernel/syscalls/statx/statx07.c
@@ -84,7 +84,7 @@ static int get_mode(char *file_name, int flag_type, char *flag_name)
 	return buf.stx_mode;
 }
 
-const struct test_cases {
+static const struct test_cases {
 	int flag;
 	char *flag_name;
 	char *server_file;
-- 
2.33.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/3] tools/sparse: Add static check

[Petr Vorel]

Hi Richie,

> This was adapted from Sparse's inbuilt check_duplicates (-Wdecl). The
> original check appears to print a warning whenever a symbol is
> non-static, but has no prototype. It appears to work because library
> symbols are usually declared first in a header file and then again
> with their definition in a source file.

> The LTP version also checks for the various library prefixes, but
> should otherwise be the same.
LGTM, thanks!

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 2/3] doc: Add LTP-003 and LTP-004 static and tst API prefix rules

[Petr Vorel]

Hi Richie,

...
> +2.3 LTP-003: Externally visible library symbols have the tst_ prefix
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +Functions, types and variables in the public test API should have the
> +tst_ prefix. With some exceptions for symbols already prefixed with
> +safe_ or ltp_.
BTW It'd be nice to have some check for shell library (maybe shellcheck would be
able to do it).

...

> +2.1.1 LTP-004: Test executable symbols are marked static
> +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> +
> +Test executables should not export symbols unecessarily. This means
typo s/unecessarily/unnecessarily/

> +that all top-level variables and functions should be marked with the
> +static keyword. The only visible symbols should be those included from
> +shared object files.
> +

Reviewed-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 3/3] statx: Add missing static keyword to tcase variable

[Petr Vorel]

Hi Richie,

obviously correct. Thanks!

Reviewed-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/3] tools/sparse: Add static check

[Richard Palethorpe]

Hello,

Richard Palethorpe <rpalethorpe@suse.com> writes:

> This was adapted from Sparse's inbuilt check_duplicates (-Wdecl). The
> original check appears to print a warning whenever a symbol is
> non-static, but has no prototype. It appears to work because library
> symbols are usually declared first in a header file and then again
> with their definition in a source file.
>
> The LTP version also checks for the various library prefixes, but
> should otherwise be the same.
>
> Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
> ---
>  tools/sparse/sparse-ltp.c | 53 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 53 insertions(+)
>
> diff --git a/tools/sparse/sparse-ltp.c b/tools/sparse/sparse-ltp.c
> index 45874e8eb..73725d191 100644
> --- a/tools/sparse/sparse-ltp.c
> +++ b/tools/sparse/sparse-ltp.c
> @@ -82,6 +82,57 @@ static void do_entrypoint_checks(struct entrypoint *ep)
>  	} END_FOR_EACH_PTR(bb);
>  }
>  
> +/* Check for LTP-003 and LTP-004
> + *
> + * Try to find cases where the static keyword was forgotten.
> + */
> +static void check_symbol_visibility(struct symbol *sym)
> +{
> +	struct symbol *next = sym;

Ooops, we don't need next anymore.

-- 
Thank you,
Richard.

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/3] tools/sparse: Add static check

[Cyril Hrubis]

Hi!
Looks good to me as well.

Acked-by: Cyril Hrubis <chrubis@suse.cz>

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 3/3] statx: Add missing static keyword to tcase variable

[Cyril Hrubis]

Hi!
> obviously correct. Thanks!
> 
> Reviewed-by: Petr Vorel <pvorel@suse.cz>

Second to this.

Reviewed-by: Cyril Hrubis <chrubis@suse.cz>

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 2/3] doc: Add LTP-003 and LTP-004 static and tst API prefix rules

[Cyril Hrubis]

Hi!
> BTW It'd be nice to have some check for shell library (maybe shellcheck would be
> able to do it).

Actually the check is in the shell library script.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 2/3] doc: Add LTP-003 and LTP-004 static and tst API prefix rules

[Petr Vorel]

> Hi!
> > BTW It'd be nice to have some check for shell library (maybe shellcheck would be
> > able to do it).

> Actually the check is in the shell library script.
I'm avare only about the check in tst_run() which check that tests are not using
variables starting with TST or _tst_ prefix, which is important.

My remark was about "LTP-003: Externally visible library symbols have the tst_
prefix", which is slightly different think. Not sure if can be scripted, thus
feel free to ignore it.

I have more remarks on shell checking, but post it as a separate thread.

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 2/3] doc: Add LTP-003 and LTP-004 static and tst API prefix rules

[Cyril Hrubis]

Hi!
Withe the typos fixed:

Reviewed-by: Cyril Hrubis <chrubis@suse.cz>

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/3] tools/sparse: Add static check

[Richard Palethorpe]

Hello Petr and Cyril,

Thanks! pushed. Please pull and try it out.

Petr Vorel <pvorel@suse.cz> writes:

> Hi Richie,
>
>> This was adapted from Sparse's inbuilt check_duplicates (-Wdecl). The
>> original check appears to print a warning whenever a symbol is
>> non-static, but has no prototype. It appears to work because library
>> symbols are usually declared first in a header file and then again
>> with their definition in a source file.
>
>> The LTP version also checks for the various library prefixes, but
>> should otherwise be the same.
> LGTM, thanks!

I took the liberty (;-)) of interpreting this as Acked-by.

>
> Kind regards,
> Petr


-- 
Thank you,
Richard.

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/3] tools/sparse: Add static check

[Cyril Hrubis]

Hi!
> Thanks! pushed. Please pull and try it out.

Looks like it fails on fuzzy sync since it uses tst_ but it's in an
header.

These definitions should be static inline and changing them so fixes the
warnings. It looks like static inline functions does not make it into
the symbol test at all.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/3] tools/sparse: Add static check

[Cyril Hrubis]

Hi!
> > Thanks! pushed. Please pull and try it out.
> 
> Looks like it fails on fuzzy sync since it uses tst_ but it's in an
> header.
> 
> These definitions should be static inline and changing them so fixes the
> warnings. It looks like static inline functions does not make it into
> the symbol test at all.

This is even stranger, the 'static inline void' functions does not make
it into the check function, but anything that returns a non-void value
does get there so we need:

diff --git a/tools/sparse/sparse-ltp.c b/tools/sparse/sparse-ltp.c
index 2f32bfa38..b1677d336 100644
--- a/tools/sparse/sparse-ltp.c
+++ b/tools/sparse/sparse-ltp.c
@@ -98,7 +98,7 @@ static void check_symbol_visibility(const struct symbol *const sym)
        if (!(mod & MOD_TOPLEVEL))
                return;

-       if (has_lib_prefix && (mod & MOD_STATIC)) {
+       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).",
                        name);

And:

diff --git a/include/tst_fuzzy_sync.h b/include/tst_fuzzy_sync.h
index 8f97bb8f6..bc3450294 100644
--- a/include/tst_fuzzy_sync.h
+++ b/include/tst_fuzzy_sync.h
@@ -210,7 +210,7 @@ struct tst_fzsync_pair {
  *
  * @sa tst_fzsync_pair_reset()
  */
-static void tst_fzsync_pair_init(struct tst_fzsync_pair *pair)
+static inline void tst_fzsync_pair_init(struct tst_fzsync_pair *pair)
 {
        CHK(avg_alpha, 0, 1, 0.25);
        CHK(min_samples, 20, INT_MAX, 1024);
@@ -230,7 +230,7 @@ static void tst_fzsync_pair_init(struct tst_fzsync_pair *pair)
  *
  * Call this from your cleanup function.
  */
-static void tst_fzsync_pair_cleanup(struct tst_fzsync_pair *pair)
+static inline void tst_fzsync_pair_cleanup(struct tst_fzsync_pair *pair)
 {
        if (pair->thread_b) {
                /* Revoke thread B if parent hits accidental break */
@@ -254,7 +254,7 @@ struct tst_fzsync_run_thread {
  * Wrap run_b for tst_fzsync_pair_reset to enable pthread cancel
  * at the start of the thread B.
  */
-static void *tst_fzsync_thread_wrapper(void *run_thread)
+static inline void *tst_fzsync_thread_wrapper(void *run_thread)
 {
        struct tst_fzsync_run_thread t = *(struct tst_fzsync_run_thread *)run_thread;

@@ -268,7 +268,7 @@ static void *tst_fzsync_thread_wrapper(void *run_thread)
  *
  * @relates tst_fzsync_stat
  */
-static void tst_init_stat(struct tst_fzsync_stat *s)
+static inline void tst_init_stat(struct tst_fzsync_stat *s)
 {
        s->avg = 0;
        s->avg_dev = 0;
@@ -292,7 +292,7 @@ static void tst_init_stat(struct tst_fzsync_stat *s)
  *
  * @sa tst_fzsync_pair_init()
  */
-static void tst_fzsync_pair_reset(struct tst_fzsync_pair *pair,
+static inline void tst_fzsync_pair_reset(struct tst_fzsync_pair *pair,
                                  void *(*run_b)(void *))
 {
        tst_fzsync_pair_cleanup(pair);
@@ -340,7 +340,7 @@ static inline void tst_fzsync_stat_info(struct tst_fzsync_stat stat,
  *
  * @relates tst_fzsync_pair
  */
-static void tst_fzsync_pair_info(struct tst_fzsync_pair *pair)
+static inline void tst_fzsync_pair_info(struct tst_fzsync_pair *pair)
 {
        tst_res(TINFO, "loop = %d, delay_bias = %d",
                pair->exec_loop, pair->delay_bias);
@@ -493,7 +493,7 @@ static inline void tst_upd_diff_stat(struct tst_fzsync_stat *s,
  *
  * @relates tst_fzsync_pair
  */
-static void tst_fzsync_pair_update(struct tst_fzsync_pair *pair)
+static inline void tst_fzsync_pair_update(struct tst_fzsync_pair *pair)
 {
        float alpha = pair->avg_alpha;
        float per_spin_time, time_delay;



-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/3] tools/sparse: Add static check

[Richard Palethorpe]

Hello,

Cyril Hrubis <chrubis@suse.cz> writes:

> Hi!
>> > Thanks! pushed. Please pull and try it out.
>> 
>> Looks like it fails on fuzzy sync since it uses tst_ but it's in an
>> header.
>> 
>> These definitions should be static inline and changing them so fixes the
>> warnings. It looks like static inline functions does not make it into
>> the symbol test at all.

Ah, I vaguely remember now that "static inline" is the correct way to
include functions in header files. Otherwise they shouldn't be in header
files. We have LTO now as well so possibly Fuzzy sync shouldn't be all
in the header, but that's another matter.

>
> This is even stranger, the 'static inline void' functions does not make
> it into the check function, but anything that returns a non-void value
> does get there so we need:

I have no idea...

>
> diff --git a/tools/sparse/sparse-ltp.c b/tools/sparse/sparse-ltp.c
> index 2f32bfa38..b1677d336 100644
> --- a/tools/sparse/sparse-ltp.c
> +++ b/tools/sparse/sparse-ltp.c
> @@ -98,7 +98,7 @@ static void check_symbol_visibility(const struct symbol *const sym)
>         if (!(mod & MOD_TOPLEVEL))
>                 return;
>
> -       if (has_lib_prefix && (mod & MOD_STATIC)) {
> +       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).",
>                         name);
>

OK, this loooks good and below. Could you send a patch? Also can add

Reviewed-by: Richard Palethorpe <rpalethorpe@suse.com>

> And:
>
> diff --git a/include/tst_fuzzy_sync.h b/include/tst_fuzzy_sync.h
> index 8f97bb8f6..bc3450294 100644
> --- a/include/tst_fuzzy_sync.h
> +++ b/include/tst_fuzzy_sync.h
> @@ -210,7 +210,7 @@ struct tst_fzsync_pair {
>   *
>   * @sa tst_fzsync_pair_reset()
>   */
> -static void tst_fzsync_pair_init(struct tst_fzsync_pair *pair)
> +static inline void tst_fzsync_pair_init(struct tst_fzsync_pair *pair)
>  {
>         CHK(avg_alpha, 0, 1, 0.25);
>         CHK(min_samples, 20, INT_MAX, 1024);
> @@ -230,7 +230,7 @@ static void tst_fzsync_pair_init(struct tst_fzsync_pair *pair)
>   *
>   * Call this from your cleanup function.
>   */
> -static void tst_fzsync_pair_cleanup(struct tst_fzsync_pair *pair)
> +static inline void tst_fzsync_pair_cleanup(struct tst_fzsync_pair *pair)
>  {
>         if (pair->thread_b) {
>                 /* Revoke thread B if parent hits accidental break */
> @@ -254,7 +254,7 @@ struct tst_fzsync_run_thread {
>   * Wrap run_b for tst_fzsync_pair_reset to enable pthread cancel
>   * at the start of the thread B.
>   */
> -static void *tst_fzsync_thread_wrapper(void *run_thread)
> +static inline void *tst_fzsync_thread_wrapper(void *run_thread)
>  {
>         struct tst_fzsync_run_thread t = *(struct tst_fzsync_run_thread *)run_thread;
>
> @@ -268,7 +268,7 @@ static void *tst_fzsync_thread_wrapper(void *run_thread)
>   *
>   * @relates tst_fzsync_stat
>   */
> -static void tst_init_stat(struct tst_fzsync_stat *s)
> +static inline void tst_init_stat(struct tst_fzsync_stat *s)
>  {
>         s->avg = 0;
>         s->avg_dev = 0;
> @@ -292,7 +292,7 @@ static void tst_init_stat(struct tst_fzsync_stat *s)
>   *
>   * @sa tst_fzsync_pair_init()
>   */
> -static void tst_fzsync_pair_reset(struct tst_fzsync_pair *pair,
> +static inline void tst_fzsync_pair_reset(struct tst_fzsync_pair *pair,
>                                   void *(*run_b)(void *))
>  {
>         tst_fzsync_pair_cleanup(pair);
> @@ -340,7 +340,7 @@ static inline void tst_fzsync_stat_info(struct tst_fzsync_stat stat,
>   *
>   * @relates tst_fzsync_pair
>   */
> -static void tst_fzsync_pair_info(struct tst_fzsync_pair *pair)
> +static inline void tst_fzsync_pair_info(struct tst_fzsync_pair *pair)
>  {
>         tst_res(TINFO, "loop = %d, delay_bias = %d",
>                 pair->exec_loop, pair->delay_bias);
> @@ -493,7 +493,7 @@ static inline void tst_upd_diff_stat(struct tst_fzsync_stat *s,
>   *
>   * @relates tst_fzsync_pair
>   */
> -static void tst_fzsync_pair_update(struct tst_fzsync_pair *pair)
> +static inline void tst_fzsync_pair_update(struct tst_fzsync_pair *pair)
>  {
>         float alpha = pair->avg_alpha;
>         float per_spin_time, time_delay;


-- 
Thank you,
Richard.

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/3] tools/sparse: Add static check

[Cyril Hrubis]

Hi!
> >> > Thanks! pushed. Please pull and try it out.
> >> 
> >> Looks like it fails on fuzzy sync since it uses tst_ but it's in an
> >> header.
> >> 
> >> These definitions should be static inline and changing them so fixes the
> >> warnings. It looks like static inline functions does not make it into
> >> the symbol test at all.
> 
> Ah, I vaguely remember now that "static inline" is the correct way to
> include functions in header files. Otherwise they shouldn't be in header
> files. We have LTO now as well so possibly Fuzzy sync shouldn't be all
> in the header, but that's another matter.

If I recall correctly LTO is currently broken on ARM and even if that
was working we still have to support 10 years old compilers anyways...

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/3] tools/sparse: Add static check

[Cyril Hrubis]

Hi!
I guess that we can also silence some of the useless output by skipping
some old library crap as:

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).",

Will send another patch.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp