diff mbox series

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

Message ID 20200422003735.3891-6-rosenp@gmail.com (mailing list archive)
State New, archived
Headers show
Series [01/12] utils: fix compilation with C++98 | expand

Commit Message

Rosen Penev April 22, 2020, 12:37 a.m. UTC 
Found with -Wmissing-noreturn and -Wunreachable-code-return

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 utils/v4l2-compliance/v4l2-compliance.cpp | 9 ++-------
 utils/v4l2-dbg/v4l2-dbg.cpp               | 7 +------
 2 files changed, 3 insertions(+), 13 deletions(-)

Comments

Hans Verkuil April 23, 2020, 8:34 a.m. UTC | #1 
On 22/04/2020 02:37, Rosen Penev wrote:
> Found with -Wmissing-noreturn and -Wunreachable-code-return
> 
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
>  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;
>  		}
>  	}
>  
>
diff mbox series

Patch

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");
@@ -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;
 		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;
 		}
 	}