[libcamera-devel] Avoid Segmentation Fault when use --help parameter

Message ID 20190603015749.1311-1-eamanu@eamanu.com
State Superseded
Headers show
Series
  • [libcamera-devel] Avoid Segmentation Fault when use --help parameter
Related show

Commit Message

Emmanuel Arias June 3, 2019, 1:57 a.m. UTC
When Cam::parseOptions receive help args return -EINTR and
Cam::init return 0 to main. But the first if does not return EXIT_FAILURE
and continue. That launch a SegFaults
---
 src/cam/main.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Jacopo Mondi June 3, 2019, 6:24 a.m. UTC | #1
Hello Emanuel,
   thanks for the patch.

I see a segfault as well when calling cam with the --help option. NIce
catch!

On Sun, Jun 02, 2019 at 10:57:49PM -0300, Emmanuel Arias wrote:
> When Cam::parseOptions receive help args return -EINTR and
> Cam::init return 0 to main. But the first if does not return EXIT_FAILURE
> and continue. That launch a SegFaults
> ---
>  src/cam/main.cpp | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/cam/main.cpp b/src/cam/main.cpp
> index dbf0491..d23390a 100644
> --- a/src/cam/main.cpp
> +++ b/src/cam/main.cpp
> @@ -61,7 +61,7 @@ int CamApp::init(int argc, char **argv)
>
>  	ret = parseOptions(argc, argv);
>  	if (ret < 0)
> -		return ret == -EINTR ? 0 : ret;
> +		return ret == -EINTR ? -EINTR : ret;

If ret == -EINTR you return -EINTR, while otherwise you return ret.
Isn't this equivalent to 'return ret' directly :) ?

Furthermore, the main loop that calls CamApp::init() handles the
return code as:

	if (app.init(argc, argv))
		return EXIT_FAILURE;

So you would EXIT_FAILURE on --help, which is not nice.

How about returning ret from CamApp::init() and handle the return code
in the main loop with:

-       if (app.init(argc, argv))
-               return EXIT_FAILURE;
+       int ret = app.init(argc, argv);
+       if (ret)
+               return ret == -EINTR ? 0 : EXIT_FAILURE;

>
>  	cm_ = CameraManager::instance();
>
> @@ -196,7 +196,7 @@ int main(int argc, char **argv)
>
>  	if (app.init(argc, argv))
>  		return EXIT_FAILURE;
> -
> +

This is an unrelated change, so please drop it.

Thank you
    j

>  	struct sigaction sa = {};
>  	sa.sa_handler = &signalHandler;
>  	sigaction(SIGINT, &sa, nullptr);
> --
> 2.11.0
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

Patch

diff --git a/src/cam/main.cpp b/src/cam/main.cpp
index dbf0491..d23390a 100644
--- a/src/cam/main.cpp
+++ b/src/cam/main.cpp
@@ -61,7 +61,7 @@  int CamApp::init(int argc, char **argv)
 
 	ret = parseOptions(argc, argv);
 	if (ret < 0)
-		return ret == -EINTR ? 0 : ret;
+		return ret == -EINTR ? -EINTR : ret;
 
 	cm_ = CameraManager::instance();
 
@@ -196,7 +196,7 @@  int main(int argc, char **argv)
 
 	if (app.init(argc, argv))
 		return EXIT_FAILURE;
-
+	
 	struct sigaction sa = {};
 	sa.sa_handler = &signalHandler;
 	sigaction(SIGINT, &sa, nullptr);