[libcamera-devel] cam: Fix cam --help crash
diff mbox series

Message ID 20190610114936.30175-1-niklas.soderlund@ragnatech.se
State Accepted
Commit 238919be5949a0bc632920cbd5b58cc4a5ca4678
Headers show
Series
  • [libcamera-devel] cam: Fix cam --help crash
Related show

Commit Message

Niklas Söderlund June 10, 2019, 11:49 a.m. UTC 
The cam utility do not terminate correctly if invoked with only --help.
It prints the help information and then segfaults as the application is
not terminated correctly. Fix this by moving the return code check of
the option parsing to main().

Reported-by: Emmanuel Arias <eamanu@eamanu.com>
Suggested-by: Jacopo Mondi <jacopo@jmondi.org>
Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
---
 src/cam/main.cpp | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

Comments

Kieran Bingham June 10, 2019, 12:14 p.m. UTC | #1 
Hi Niklas,

Thanks for picking this up.

On 10/06/2019 12:49, Niklas Söderlund wrote:
> The cam utility do not terminate correctly if invoked with only --help.

'does not'

> It prints the help information and then segfaults as the application is
> not terminated correctly. Fix this by moving the return code check of

"segfaults due to the application incorrectly handling the return value." ?

> the option parsing to main().
> 
> Reported-by: Emmanuel Arias <eamanu@eamanu.com>
> Suggested-by: Jacopo Mondi <jacopo@jmondi.org>

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> ---
>  src/cam/main.cpp | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/src/cam/main.cpp b/src/cam/main.cpp
> index dbf04917bcc5aa38..f03a9faf87fac865 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;
>  
>  	cm_ = CameraManager::instance();
>  
> @@ -193,9 +193,11 @@ void signalHandler(int signal)
>  int main(int argc, char **argv)
>  {
>  	CamApp app;
> +	int ret;
>  
> -	if (app.init(argc, argv))
> -		return EXIT_FAILURE;
> +	ret = app.init(argc, argv);
> +	if (ret)
> +		return ret == -EINTR ? 0 : EXIT_FAILURE;
>  
>  	struct sigaction sa = {};
>  	sa.sa_handler = &signalHandler;
>
Laurent Pinchart June 10, 2019, 2:27 p.m. UTC | #2 
Hello,

On Mon, Jun 10, 2019 at 01:14:25PM +0100, Kieran Bingham wrote:
> Hi Niklas,
> 
> Thanks for picking this up.
> 
> On 10/06/2019 12:49, Niklas Söderlund wrote:
> > The cam utility do not terminate correctly if invoked with only --help.
> 
> 'does not'
> 
> > It prints the help information and then segfaults as the application is
> > not terminated correctly. Fix this by moving the return code check of
> 
> "segfaults due to the application incorrectly handling the return value." ?
> 
> > the option parsing to main().
> > 
> > Reported-by: Emmanuel Arias <eamanu@eamanu.com>
> > Suggested-by: Jacopo Mondi <jacopo@jmondi.org>
> 
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 
> > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> > ---
> >  src/cam/main.cpp | 8 +++++---
> >  1 file changed, 5 insertions(+), 3 deletions(-)
> > 
> > diff --git a/src/cam/main.cpp b/src/cam/main.cpp
> > index dbf04917bcc5aa38..f03a9faf87fac865 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;
> >  
> >  	cm_ = CameraManager::instance();
> >  
> > @@ -193,9 +193,11 @@ void signalHandler(int signal)
> >  int main(int argc, char **argv)
> >  {
> >  	CamApp app;
> > +	int ret;
> >  
> > -	if (app.init(argc, argv))
> > -		return EXIT_FAILURE;
> > +	ret = app.init(argc, argv);
> > +	if (ret)
> > +		return ret == -EINTR ? 0 : EXIT_FAILURE;
> >  
> >  	struct sigaction sa = {};
> >  	sa.sa_handler = &signalHandler;
Niklas Söderlund June 10, 2019, 9:08 p.m. UTC | #3 
Hi,

Thanks for reviewing this, I have pushed this to master with Kieran's 
spelling improvements to the commit message.

On 2019-06-10 17:27:14 +0300, Laurent Pinchart wrote:
> Hello,
> 
> On Mon, Jun 10, 2019 at 01:14:25PM +0100, Kieran Bingham wrote:
> > Hi Niklas,
> > 
> > Thanks for picking this up.
> > 
> > On 10/06/2019 12:49, Niklas Söderlund wrote:
> > > The cam utility do not terminate correctly if invoked with only --help.
> > 
> > 'does not'
> > 
> > > It prints the help information and then segfaults as the application is
> > > not terminated correctly. Fix this by moving the return code check of
> > 
> > "segfaults due to the application incorrectly handling the return value." ?
> > 
> > > the option parsing to main().
> > > 
> > > Reported-by: Emmanuel Arias <eamanu@eamanu.com>
> > > Suggested-by: Jacopo Mondi <jacopo@jmondi.org>
> > 
> > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> > 
> > > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> > > ---
> > >  src/cam/main.cpp | 8 +++++---
> > >  1 file changed, 5 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/src/cam/main.cpp b/src/cam/main.cpp
> > > index dbf04917bcc5aa38..f03a9faf87fac865 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;
> > >  
> > >  	cm_ = CameraManager::instance();
> > >  
> > > @@ -193,9 +193,11 @@ void signalHandler(int signal)
> > >  int main(int argc, char **argv)
> > >  {
> > >  	CamApp app;
> > > +	int ret;
> > >  
> > > -	if (app.init(argc, argv))
> > > -		return EXIT_FAILURE;
> > > +	ret = app.init(argc, argv);
> > > +	if (ret)
> > > +		return ret == -EINTR ? 0 : EXIT_FAILURE;
> > >  
> > >  	struct sigaction sa = {};
> > >  	sa.sa_handler = &signalHandler;
> 
> -- 
> Regards,
> 
> Laurent Pinchart

Patch
diff mbox series 

diff --git a/src/cam/main.cpp b/src/cam/main.cpp
index dbf04917bcc5aa38..f03a9faf87fac865 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;
 
 	cm_ = CameraManager::instance();
 
@@ -193,9 +193,11 @@  void signalHandler(int signal)
 int main(int argc, char **argv)
 {
 	CamApp app;
+	int ret;
 
-	if (app.init(argc, argv))
-		return EXIT_FAILURE;
+	ret = app.init(argc, argv);
+	if (ret)
+		return ret == -EINTR ? 0 : EXIT_FAILURE;
 
 	struct sigaction sa = {};
 	sa.sa_handler = &signalHandler;