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

[Cyril Hrubis <chrubis@suse.cz>]

Hi!
> +#define err(...) ({ \
> +	fprintf(stderr, __VA_ARGS__); \
> +	fprintf(stderr, "\n"); \
> +	usage(); \
> +	return 2; })
           ^
This should rather be exit(2);

It's only matter of time until someone uses that in a function outside
of main.

> +#define fail(...) ({ \
> +	fprintf(stderr, __VA_ARGS__); \
> +	fprintf(stderr, "\n"); \
> +	return 1; })

Here as well.

> +#define info(...) ({ \
> +	fprintf(stderr, __VA_ARGS__); \
> +	fprintf(stderr, "\n"); \
> +	return 0; })

The naming here is a bit of strange, I wouldn't expect that function
called info() would exit the process.

Maybe these three should include exit in the function name such as
info_exit(), err_exit() and fail_exit().

>  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
> 

-- 
Cyril Hrubis
chrubis@suse.cz

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