Re: [PATCH 06/12] utils: add noreturn attribute and remove dead code

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 22/04/2020 02:37, Rosen Penev wrote:
> Found with -Wmissing-noreturn and -Wunreachable-code-return
> 
> Signed-off-by: Rosen Penev <rosenp@xxxxxxxxx>
> ---
>  utils/v4l2-compliance/v4l2-compliance.cpp | 9 ++-------
>  utils/v4l2-dbg/v4l2-dbg.cpp               | 7 +------
>  2 files changed, 3 insertions(+), 13 deletions(-)
> 
> diff --git a/utils/v4l2-compliance/v4l2-compliance.cpp b/utils/v4l2-compliance/v4l2-compliance.cpp
> index b3a18492..fbf34914 100644
> --- a/utils/v4l2-compliance/v4l2-compliance.cpp
> +++ b/utils/v4l2-compliance/v4l2-compliance.cpp
> @@ -155,6 +155,7 @@ static struct option long_options[] = {
>  	{0, 0, 0, 0}
>  };
>  
> +__attribute__((noreturn))
>  static void usage()
>  {
>  	printf("Usage:\n");

I think usage() shouldn't exit, instead leave that to the caller.

> @@ -482,6 +483,7 @@ static void restoreState()
>  	node->reopen();
>  }
>  
> +__attribute__((noreturn))
>  static void signal_handler_interrupt(int signum)
>  {
>  	restoreState();
> @@ -1544,7 +1546,6 @@ int main(int argc, char **argv)
>  		switch (ch) {
>  		case OptHelp:
>  			usage();
> -			return 0;

So instead of doing 'return 0;', do 'exit(0);' here and exit(1) elsewhere.

Clearly in many cases I already assumed that usage() would return, so let's
change usage() accordingly.

Same for v4l2-dbg.cpp.

Regards,

	Hans

>  		case OptSetDevice:
>  			device = make_devname(optarg, "video", media_bus_info);
>  			break;
> @@ -1619,7 +1620,6 @@ int main(int argc, char **argv)
>  						color_component = 2;
>  					else {
>  						usage();
> -						exit(1);
>  					}
>  					break;
>  				case 1:
> @@ -1634,7 +1634,6 @@ int main(int argc, char **argv)
>  					break;
>  				default:
>  					usage();
> -					exit(1);
>  				}
>  			}
>  			break;
> @@ -1647,7 +1646,6 @@ int main(int argc, char **argv)
>  				show_colors = isatty(STDOUT_FILENO);
>  			else {
>  				usage();
> -				exit(1);
>  			}
>  			break;
>  		case OptNoWarnings:
> @@ -1669,13 +1667,11 @@ int main(int argc, char **argv)
>  			fprintf(stderr, "Option `%s' requires a value\n",
>  				argv[optind]);
>  			usage();
> -			return 1;
>  		case '?':
>  			if (argv[optind])
>  				fprintf(stderr, "Unknown argument `%s'\n",
>  					argv[optind]);
>  			usage();
> -			return 1;
>  		}
>  	}
>  	if (optind < argc) {
> @@ -1684,7 +1680,6 @@ int main(int argc, char **argv)
>  			fprintf(stderr, "%s ", argv[optind++]);
>  		fprintf(stderr, "\n");
>  		usage();
> -		return 1;
>  	}
>  	bool direct = !options[OptUseWrapper];
>  	int fd;
> diff --git a/utils/v4l2-dbg/v4l2-dbg.cpp b/utils/v4l2-dbg/v4l2-dbg.cpp
> index cd387d1d..7b986a50 100644
> --- a/utils/v4l2-dbg/v4l2-dbg.cpp
> +++ b/utils/v4l2-dbg/v4l2-dbg.cpp
> @@ -162,6 +162,7 @@ static struct option long_options[] = {
>  	{0, 0, 0, 0}
>  };
>  
> +__attribute__((noreturn))
>  static void usage()
>  {
>  	printf("Usage: v4l2-dbg [options] [values]\n"
> @@ -387,13 +388,11 @@ static int parse_subopt(char **subs, const char * const *subopts, char **value)
>  	if (opt == -1) {
>  		fprintf(stderr, "Invalid suboptions specified\n");
>  		usage();
> -		exit(1);
>  	}
>  	if (*value == NULL) {
>  		fprintf(stderr, "No value given to suboption <%s>\n",
>  				subopts[opt]);
>  		usage();
> -		exit(1);
>  	}
>  	return opt;
>  }
> @@ -429,7 +428,6 @@ int main(int argc, char **argv)
>  
>  	if (argc == 1) {
>  		usage();
> -		return 0;
>  	}
>  	for (i = 0; long_options[i].name; i++) {
>  		if (!isalpha(long_options[i].val))
> @@ -467,7 +465,6 @@ int main(int argc, char **argv)
>  		switch (ch) {
>  		case OptHelp:
>  			usage();
> -			return 0;
>  
>  		case OptSetDevice:
>  			device = optarg;
> @@ -538,13 +535,11 @@ int main(int argc, char **argv)
>  			fprintf(stderr, "Option `%s' requires a value\n",
>  				argv[optind]);
>  			usage();
> -			return 1;
>  
>  		case '?':
>  			fprintf(stderr, "Unknown argument `%s'\n",
>  				argv[optind]);
>  			usage();
> -			return 1;
>  		}
>  	}
>  
> 




[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux