[PATCH] staging : media : atomisp : input_system.c : fixes the error of control reaches end of non-void function [-Werror=return-type]

[PATCH] staging : media : atomisp : input_system.c : fixes the error of control reaches end of non-void function [-Werror=return-type]

[Kushal Kothari]

The compiler cannot tell from that code if the function will ever reach
the end and still return something. To make that clear, replaced the last
line with just "else" which fixes the error of "error: control reaches end of non-void function [-Werror=return-type]"

Signed-off-by: Kushal Kothari <kushalkothari285@gmail.com>
---
 .../media/atomisp/pci/hive_isp_css_common/host/input_system.c    | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_system.c b/drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_system.c
index 8e085dda0c18..91bad8f34efc 100644
--- a/drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_system.c
+++ b/drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_system.c
@@ -1587,6 +1587,7 @@ static input_system_err_t input_system_configure_channel_sensor(
 	status = set_source_type(&config.source_type, channel.source_type,
 				 &config.source_type_flags);
 	if (status != INPUT_SYSTEM_ERR_NO_ERROR) return status;
+	else return status;
 
 	// Check for conflicts on source (implicitly on multicast, capture unit and input buffer).
 
-- 
2.25.1

Re: [Outreachy kernel] [PATCH] staging : media : atomisp : input_system.c : fixes the error of control reaches end of non-void function [-Werror=return-type]

[Alison Schofield]

On Fri, Oct 15, 2021 at 04:00:09AM +0530, Kushal Kothari wrote:
> The compiler cannot tell from that code if the function will ever reach
> the end and still return something. To make that clear, replaced the last
> line with just "else" which fixes the error of "error: control reaches end of non-void function [-Werror=return-type]"
> 
> Signed-off-by: Kushal Kothari <kushalkothari285@gmail.com>

Hi Kushal,

Thanks for the patch and welcome to Outreachy.

Unfortunately we're not accepting patches in this driver this round.
Any other driver within media is OK. See the google-groups welcome
message and the email history since October 8th for more info.
I also updated the first patch tutorial, because you showed me that
this restriction was too easy to miss.

Anyway, just so your patch isn't for nothing, I'll give you feedback
on the commit message. You'll want to write that in the imperative mode,
So, for one like this, maybe something like:

'remove conditional from return in non-void function'

Alison


> ---
>  .../media/atomisp/pci/hive_isp_css_common/host/input_system.c    | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_system.c b/drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_system.c
> index 8e085dda0c18..91bad8f34efc 100644
> --- a/drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_system.c
> +++ b/drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_system.c
> @@ -1587,6 +1587,7 @@ static input_system_err_t input_system_configure_channel_sensor(
>  	status = set_source_type(&config.source_type, channel.source_type,
>  				 &config.source_type_flags);
>  	if (status != INPUT_SYSTEM_ERR_NO_ERROR) return status;
> +	else return status;
>  
>  	// Check for conflicts on source (implicitly on multicast, capture unit and input buffer).
>  
> -- 
> 2.25.1
> 
> -- 
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20211014223007.596789-1-kushalkothari285%40gmail.com.

Re: [Outreachy kernel] [PATCH] staging : media : atomisp : input_system.c : fixes the error of control reaches end of non-void function [-Werror=return-type]

[Kieran Bingham]

Hi Kushal,

Quoting Alison Schofield (2021-10-15 03:26:59)
> On Fri, Oct 15, 2021 at 04:00:09AM +0530, Kushal Kothari wrote:
> > The compiler cannot tell from that code if the function will ever reach
> > the end and still return something. To make that clear, replaced the last
> > line with just "else" which fixes the error of "error: control reaches end of non-void function [-Werror=return-type]"
> > 
> > Signed-off-by: Kushal Kothari <kushalkothari285@gmail.com>
> 
> Hi Kushal,
> 
> Thanks for the patch and welcome to Outreachy.
> 
> Unfortunately we're not accepting patches in this driver this round.
> Any other driver within media is OK. See the google-groups welcome
> message and the email history since October 8th for more info.
> I also updated the first patch tutorial, because you showed me that
> this restriction was too easy to miss.
> 
> Anyway, just so your patch isn't for nothing, I'll give you feedback
> on the commit message. You'll want to write that in the imperative mode,
> So, for one like this, maybe something like:
> 
> 'remove conditional from return in non-void function'
> 
> Alison
> 
> 
> > ---
> >  .../media/atomisp/pci/hive_isp_css_common/host/input_system.c    | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_system.c b/drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_system.c
> > index 8e085dda0c18..91bad8f34efc 100644
> > --- a/drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_system.c
> > +++ b/drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_system.c
> > @@ -1587,6 +1587,7 @@ static input_system_err_t input_system_configure_channel_sensor(
> >       status = set_source_type(&config.source_type, channel.source_type,
> >                                &config.source_type_flags);
> >       if (status != INPUT_SYSTEM_ERR_NO_ERROR) return status;
> > +     else return status;

I see the mention that outreachy are not taking patches to this driver
currently, but I saw this and thought it might still be helpful to
highlight that you should look more deeply at this change, and try to
understand why it would not be possible to accept it. Given that a fix
wouldn't be accepted through outreachy, it may not be worth submitting a
new version but please make sure you examine why this would not be
correct:


You are introducing a line which would essentially cut out the rest of
the function, as the compiler would alway return at this point with your
change.
Before your change:

	If A
		return status;

	... many more lines of code ...

After your change:

	If A
		return status;
	else
		return status;

	... many more lines of now unreachable code ...
		
Good luck on your outreachy journey.

--
Kieran


> >  
> >       // Check for conflicts on source (implicitly on multicast, capture unit and input buffer).
> >  
> > -- 
> > 2.25.1
> > 
> > -- 
> > You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> > To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> > To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20211014223007.596789-1-kushalkothari285%40gmail.com.
> 
> -- 
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20211015022659.GB428161%40alison-desk.

Re: [Outreachy kernel] [PATCH] staging : media : atomisp : input_system.c : fixes the error of control reaches end of non-void function [-Werror=return-type]

[Fabio M. De Francesco]

On Friday, October 15, 2021 12:30:09 AM CEST Kushal Kothari wrote:
> The compiler cannot tell from that code if the function will ever reach
> the end and still return something. To make that clear, replaced the last
> line with just "else" which fixes the error of "error: control reaches end 
of non-void function [-Werror=return-type]"
> 
> Signed-off-by: Kushal Kothari <kushalkothari285@gmail.com>
> ---
>  .../media/atomisp/pci/hive_isp_css_common/host/input_system.c    | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/staging/media/atomisp/pci/hive_isp_css_common/host/
input_system.c b/drivers/staging/media/atomisp/pci/hive_isp_css_common/host/
input_system.c
> index 8e085dda0c18..91bad8f34efc 100644
> --- a/drivers/staging/media/atomisp/pci/hive_isp_css_common/host/
input_system.c
> +++ b/drivers/staging/media/atomisp/pci/hive_isp_css_common/host/
input_system.c
> @@ -1587,6 +1587,7 @@ static input_system_err_t 
input_system_configure_channel_sensor(
>  	status = set_source_type(&config.source_type, 
channel.source_type,
>  				 &config.source_type_flags);
>  	if (status != INPUT_SYSTEM_ERR_NO_ERROR) return status;
> +	else return status;

Hi Kushal,

Aside from what Alison said about not accepting patches in this driver this 
round, please note that this is not the correct way to address that compiler 
warning.

Think about it: now, whatever "state" you get from set_source_type(), you 
always return it to the caller of input_system_configure_channel_sensor().

By doing so, you change the logic of the function and get dead code. This 
change makes the rest of the function code unreachable and introduces a huge 
bug.

While the patch cannot be accepted for Outreachy, think of a better solution 
as you may need to make similar changes in future patches.

Thanks and welcome,

Fabio M. De Francesco

>  
>  	// Check for conflicts on source (implicitly on multicast, 
capture unit and input buffer).
>  
> -- 
> 2.25.1
> 
> -- 
> You received this message because you are subscribed to the Google Groups 
"outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
email to outreachy-kernel+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/
outreachy-kernel/20211014223007.596789-1-kushalkothari285%40gmail.com.
> 

Re: [Outreachy kernel] [PATCH] staging : media : atomisp : input_system.c : fixes the error of control reaches end of non-void function [-Werror=return-type]

[Fabio M. De Francesco]

On Friday, October 15, 2021 2:04:49 PM CEST Fabio M. De Francesco wrote:
> Hi Kushal,
> 
> Aside from what Alison said about not accepting patches in this driver this 
> round, please note that this is not the correct way to address that 
compiler 
> warning.
> 
> Think about it: now, whatever "state" you get from set_source_type(), you 
> always return it to the caller of input_system_configure_channel_sensor().
> 
> By doing so, you change the logic of the function and get dead code. This 
> change makes the rest of the function code unreachable and introduces a 
huge 
> bug.
> 
> While the patch cannot be accepted for Outreachy, think of a better 
solution 
> as you may need to make similar changes in future patches.
> 
> Thanks and welcome,
> 
> Fabio M. De Francesco

I'm sorry, but just now I saw the reply Kieran Bingham. I'm simply restating 
what he wrote, so please disregard my email.

I think that I didn't see his email because you forgot to CC linux-
staging@lists.linux.dev and linux-kernel@vger.kernel.org.

Please read again the Outreachy rules. If I'm not wrong, you should always CC 
the two mailing lists above.

Thanks,

Fabio M. De Francesco

Re: [Outreachy kernel] [PATCH] staging : media : atomisp : input_system.c : fixes the error of control reaches end of non-void function [-Werror=return-type]

[Julia Lawall]

On Fri, 15 Oct 2021, Fabio M. De Francesco wrote:

> On Friday, October 15, 2021 2:04:49 PM CEST Fabio M. De Francesco wrote:
> > Hi Kushal,
> >
> > Aside from what Alison said about not accepting patches in this driver this
> > round, please note that this is not the correct way to address that
> compiler
> > warning.
> >
> > Think about it: now, whatever "state" you get from set_source_type(), you
> > always return it to the caller of input_system_configure_channel_sensor().
> >
> > By doing so, you change the logic of the function and get dead code. This
> > change makes the rest of the function code unreachable and introduces a
> huge
> > bug.
> >
> > While the patch cannot be accepted for Outreachy, think of a better
> solution
> > as you may need to make similar changes in future patches.
> >
> > Thanks and welcome,
> >
> > Fabio M. De Francesco
>
> I'm sorry, but just now I saw the reply Kieran Bingham. I'm simply restating
> what he wrote, so please disregard my email.
>
> I think that I didn't see his email because you forgot to CC linux-
> staging@lists.linux.dev and linux-kernel@vger.kernel.org.
>
> Please read again the Outreachy rules. If I'm not wrong, you should always CC
> the two mailing lists above.

The instructions say to use the following:

 perl scripts/get_maintainer.pl --separator , --nokeywords --nogit --nogit-fallback --norolestats --nol -f {file}

Probably the --nol argument should not be there, because some staging
drivers have mailing lists that should see the patches. I don't know if it
is desirable for everything to go to linux-kernel@vger.kernel.org though.
This part of the application process is meant to be highly newbie
tolerant.

In any case, Alison can decide.

julia

>
> Thanks,
>
> Fabio M. De Francesco
>
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/2247559.FlfiXgstMM%40localhost.localdomain.
>

Re: [Outreachy kernel] [PATCH] staging : media : atomisp : input_system.c : fixes the error of control reaches end of non-void function [-Werror=return-type]

[Greg KH]

On Fri, Oct 15, 2021 at 04:52:09PM +0200, Julia Lawall wrote:
> 
> 
> On Fri, 15 Oct 2021, Fabio M. De Francesco wrote:
> 
> > On Friday, October 15, 2021 2:04:49 PM CEST Fabio M. De Francesco wrote:
> > > Hi Kushal,
> > >
> > > Aside from what Alison said about not accepting patches in this driver this
> > > round, please note that this is not the correct way to address that
> > compiler
> > > warning.
> > >
> > > Think about it: now, whatever "state" you get from set_source_type(), you
> > > always return it to the caller of input_system_configure_channel_sensor().
> > >
> > > By doing so, you change the logic of the function and get dead code. This
> > > change makes the rest of the function code unreachable and introduces a
> > huge
> > > bug.
> > >
> > > While the patch cannot be accepted for Outreachy, think of a better
> > solution
> > > as you may need to make similar changes in future patches.
> > >
> > > Thanks and welcome,
> > >
> > > Fabio M. De Francesco
> >
> > I'm sorry, but just now I saw the reply Kieran Bingham. I'm simply restating
> > what he wrote, so please disregard my email.
> >
> > I think that I didn't see his email because you forgot to CC linux-
> > staging@lists.linux.dev and linux-kernel@vger.kernel.org.
> >
> > Please read again the Outreachy rules. If I'm not wrong, you should always CC
> > the two mailing lists above.
> 
> The instructions say to use the following:
> 
>  perl scripts/get_maintainer.pl --separator , --nokeywords --nogit --nogit-fallback --norolestats --nol -f {file}
> 
> Probably the --nol argument should not be there, because some staging
> drivers have mailing lists that should see the patches. I don't know if it
> is desirable for everything to go to linux-kernel@vger.kernel.org though.
> This part of the application process is meant to be highly newbie
> tolerant.
> 
> In any case, Alison can decide.

The patches HAVE to show up on the linux-staging@vger list at the very
least, otherwise I can not take them.

thanks,

greg k-h