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