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

[Richard Palethorpe <rpalethorpe@suse.de>]

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