[LTP] [PATCH 1/3 v2] tst_supported_fs: Unify messaging

[Li Wang]

On Fri, Sep 23, 2022 at 5:09 AM Petr Vorel <pvorel@suse.cz> wrote:

> Most of the messages used fprintf() instead of tst_{res,brk}(),
> thus convert all messages to fprintf().
>
> Add macros to shorten code.
>
> Fixes: eb47b4497 ("tst_supported_fs: Support skip list when query single
> fs")
>
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
> I'm not sure about this myself. Shouldn't we rather use tst_brk() and
> tst_res() instead? It's show tst_supported_fs as command.
>

Not exactly, looking at testcase/lib/* tools, most of them are not written
by LTP standard API, some even do not include tst_test.h.

I personally think if we want more flexibility for those small programs
as auxiliary tool, we should not apply API as dogmatism for everything.

Btw, there is patch confliction when performing git-am, if you can
rebase accordingly for solving that, the whole patchset will be
great for me.

Reviewed-by: Li Wang <liwang@redhat.com>



>
>
>  testcases/lib/tst_supported_fs.c | 61 ++++++++++++++++----------------
>  1 file changed, 30 insertions(+), 31 deletions(-)
>
> diff --git a/testcases/lib/tst_supported_fs.c
> b/testcases/lib/tst_supported_fs.c
> index 26577c726..947aa4dae 100644
> --- a/testcases/lib/tst_supported_fs.c
> +++ b/testcases/lib/tst_supported_fs.c
> @@ -15,6 +15,22 @@
>  #include "tst_test.h"
>  #include "tst_fs.h"
>
> +#define err(...) ({ \
> +       fprintf(stderr, __VA_ARGS__); \
> +       fprintf(stderr, "\n"); \
> +       usage(); \
> +       return 2; })
> +
> +#define fail(...) ({ \
> +       fprintf(stderr, __VA_ARGS__); \
> +       fprintf(stderr, "\n"); \
> +       return 1; })
> +
> +#define info(...) ({ \
> +       fprintf(stderr, __VA_ARGS__); \
> +       fprintf(stderr, "\n"); \
> +       return 0; })
> +
>  static void usage(void)
>  {
>         fprintf(stderr, "Usage:\n");
> @@ -90,67 +106,50 @@ int main(int argc, char *argv[])
>                         break;
>
>                 case 'd':
> -                       if (fsname) {
> -                               fprintf(stderr,
> -                                       "Can't specify multiple paths\n");
> -                               usage();
> -                               return 2;
> -                       }
> +                       if (fsname)
> +                               err("Can't specify multiple paths");
>
>                         fsname = tst_fs_type_name(tst_fs_type(optarg));
>                         break;
>                 }
>         }
>
> -       if (fsname && !skiplist) {
> -               fprintf(stderr, "Parameter -d requires skiplist\n");
> -               usage();
> -               return 2;
> -       }
> +       if (fsname && !skiplist)
> +               err("Parameter -d requires skiplist");
>
> -       if (argc - optind > 1) {
> -               fprintf(stderr, "Can't specify multiple fs_type\n");
> -               usage();
> -               return 2;
> -       }
> +       if (argc - optind > 1)
> +               err("Can't specify multiple fs_type");
>
>         /* fs_type */
>         if (optind < argc) {
> -               if (fsname) {
> -                       fprintf(stderr, "Can't specify fs_type and -d
> together\n");
> -                       usage();
> -                       return 2;
> -               }
> +               if (fsname)
> +                       err("Can't specify fs_type and -d together");
>
>                 fsname = argv[optind];
>         }
>
>         if (fsname) {
>                 if (fsname[0] == '\0')
> -                       tst_brk(TCONF, "fs_type is empty");
> +                       err("fs_type is empty");
>
>                 if (skiplist) {
>                         if (tst_fs_in_skiplist(fsname, (const char *
> const*)skiplist))
> -                               tst_brk(TCONF, "%s is skipped", fsname);
> -                       else
> -                               tst_res(TINFO, "%s is not skipped",
> fsname);
> +                               fail("%s is skipped", fsname);
>
> -                       return 0;
> +                       info("%s is not skipped", fsname);
>                 }
>
>                 if (tst_fs_is_supported(fsname) == TST_FS_UNSUPPORTED)
> -                       tst_brk(TCONF, "%s is not supported", fsname);
> -               else
> -                       tst_res(TINFO, "%s is supported", fsname);
> +                       fail("%s is not supported", fsname);
>
> -               return 0;
> +               info("%s is supported", fsname);
>         }
>
>         /* all filesystems */
>         filesystems = tst_get_supported_fs_types((const char *
> const*)skiplist);
>
>         if (!filesystems[0])
> -               tst_brk(TCONF, "There are no supported filesystems or all
> skipped");
> +               fail("There are no supported filesystems or all skipped");
>
>         for (i = 0; filesystems[i]; i++)
>                 printf("%s\n", filesystems[i]);
> --
> 2.37.3
>
>

-- 
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20220923/602068ba/attachment.htm>