This line is unreachable, remove it.
Signed-off-by: WANG Cong <[email protected]>
Cc: James Morris <[email protected]>
Cc: Eric Paris <[email protected]>
---
diff --git a/security/selinux/ss/mls.c b/security/selinux/ss/mls.c
index b5407f1..a2f1034 100644
--- a/security/selinux/ss/mls.c
+++ b/security/selinux/ss/mls.c
@@ -544,7 +544,6 @@ int mls_compute_sid(struct context *scontext,
default:
return -EINVAL;
}
- return -EINVAL;
}
#ifdef CONFIG_NETLABEL
On Tue, 2009-12-01 at 02:41 -0500, Amerigo Wang wrote:
> This line is unreachable, remove it.
[]
> diff --git a/security/selinux/ss/mls.c b/security/selinux/ss/mls.c
> index b5407f1..a2f1034 100644
> --- a/security/selinux/ss/mls.c
> +++ b/security/selinux/ss/mls.c
> @@ -544,7 +544,6 @@ int mls_compute_sid(struct context *scontext,
> default:
> return -EINVAL;
> }
> - return -EINVAL;
> }
I think it's better to remove the default case.
Joe Perches wrote:
> On Tue, 2009-12-01 at 02:41 -0500, Amerigo Wang wrote:
>> This line is unreachable, remove it.
> []
>> diff --git a/security/selinux/ss/mls.c b/security/selinux/ss/mls.c
>> index b5407f1..a2f1034 100644
>> --- a/security/selinux/ss/mls.c
>> +++ b/security/selinux/ss/mls.c
>> @@ -544,7 +544,6 @@ int mls_compute_sid(struct context *scontext,
>> default:
>> return -EINVAL;
>> }
>> - return -EINVAL;
>> }
>
> I think it's better to remove the default case.
>
This is totally a personal taste, I think.
Either is OK. James, any comments?
On Tue, 1 Dec 2009, Cong Wang wrote:
> > > default:
> > > return -EINVAL;
> > > }
> > > - return -EINVAL;
> > > }
> >
> > I think it's better to remove the default case.
> >
>
> This is totally a personal taste, I think.
> Either is OK. James, any comments?
Leave the default: there but with a comment /* fall through */
instead of the return.
--
James Morris
<[email protected]>
On Tue, Dec 01, 2009 at 04:31:36PM +0800, Cong Wang wrote:
> Joe Perches wrote:
>> On Tue, 2009-12-01 at 02:41 -0500, Amerigo Wang wrote:
>>> This line is unreachable, remove it.
>> []
>>> diff --git a/security/selinux/ss/mls.c b/security/selinux/ss/mls.c
>>> index b5407f1..a2f1034 100644
>>> --- a/security/selinux/ss/mls.c
>>> +++ b/security/selinux/ss/mls.c
>>> @@ -544,7 +544,6 @@ int mls_compute_sid(struct context *scontext,
>>> default:
>>> return -EINVAL;
>>> }
>>> - return -EINVAL;
>>> }
>>
>> I think it's better to remove the default case.
>>
>
> This is totally a personal taste, I think.
> Either is OK. James, any comments?
I think the last unreachable return might also stop certain
versions of gcc complaining about control reaching the end of
a non void function.
regards,
dan carpenter
James Morris wrote:
> On Tue, 1 Dec 2009, Cong Wang wrote:
>
>>>> default:
>>>> return -EINVAL;
>>>> }
>>>> - return -EINVAL;
>>>> }
>>> I think it's better to remove the default case.
>>>
>> This is totally a personal taste, I think.
>> Either is OK. James, any comments?
>
> Leave the default: there but with a comment /* fall through */
> instead of the return.
>
Ok, will do, thanks.
Dan Carpenter wrote:
> On Tue, Dec 01, 2009 at 04:31:36PM +0800, Cong Wang wrote:
>> Joe Perches wrote:
>>> On Tue, 2009-12-01 at 02:41 -0500, Amerigo Wang wrote:
>>>> This line is unreachable, remove it.
>>> []
>>>> diff --git a/security/selinux/ss/mls.c b/security/selinux/ss/mls.c
>>>> index b5407f1..a2f1034 100644
>>>> --- a/security/selinux/ss/mls.c
>>>> +++ b/security/selinux/ss/mls.c
>>>> @@ -544,7 +544,6 @@ int mls_compute_sid(struct context *scontext,
>>>> default:
>>>> return -EINVAL;
>>>> }
>>>> - return -EINVAL;
>>>> }
>>> I think it's better to remove the default case.
>>>
>> This is totally a personal taste, I think.
>> Either is OK. James, any comments?
>
> I think the last unreachable return might also stop certain
> versions of gcc complaining about control reaching the end of
> a non void function.
Hmm, aren't those version of gcc buggy? Here we have return values
in all cases of 'switch', it shouldn't complain...
On Thu, 2009-12-03 at 16:39 +0800, Cong Wang wrote:
> Dan Carpenter wrote:
> > On Tue, Dec 01, 2009 at 04:31:36PM +0800, Cong Wang wrote:
> >> Joe Perches wrote:
> >>> On Tue, 2009-12-01 at 02:41 -0500, Amerigo Wang wrote:
> >>>> This line is unreachable, remove it.
> >>> []
> >>>> diff --git a/security/selinux/ss/mls.c b/security/selinux/ss/mls.c
> >>>> index b5407f1..a2f1034 100644
> >>>> --- a/security/selinux/ss/mls.c
> >>>> +++ b/security/selinux/ss/mls.c
> >>>> @@ -544,7 +544,6 @@ int mls_compute_sid(struct context *scontext,
> >>>> default:
> >>>> return -EINVAL;
> >>>> }
> >>>> - return -EINVAL;
> >>>> }
> >>> I think it's better to remove the default case.
> >>>
> >> This is totally a personal taste, I think.
> >> Either is OK. James, any comments?
> >
> > I think the last unreachable return might also stop certain
> > versions of gcc complaining about control reaching the end of
> > a non void function.
>
> Hmm, aren't those version of gcc buggy? Here we have return values
> in all cases of 'switch', it shouldn't complain...
Various versions of gcc do complain.
It's also a bit counter-expectation to
the reader to find no return at the end
of a non-void function.
As a reader, you then have to scan the
cases to find the default: case which
isn't necessarily at the end.
If it is a simple
default:
return foo;
I think it less jarring to omit
default from the switch statement
and use
return foo;
}
at the function exit.