How to fix warning 'control reaches end of non-void function'

How to fix warning 'control reaches end of non-void function'

[Alexander Beregalov]

Hi

What is it a right way to fix these warnings?

smth function()
{
	switch (var) {
	case one:
		return 1;
	default:
		BUG();
	}
}

warning: control reaches end of non-void function

Thanks!	
	

Re: How to fix warning 'control reaches end of non-void function'

[Daniel Baluta]

Hello ,

You can add a return statement at the end , even if control never reaches there.
It is interesting to know what is the standard way of avoiding this warning?

Daniel.

On Tue, Jul 29, 2008 at 10:17 PM, Alexander Beregalov
<a.beregalov@gmail.com> wrote:
> Hi
>
> What is it a right way to fix these warnings?
>
> smth function()
> {
>        switch (var) {
>        case one:
>                return 1;
>        default:
>                BUG();
>        }
> }
>
> warning: control reaches end of non-void function
>
> Thanks!
>
>
> --
> To unsubscribe from this list: send an email with
> "unsubscribe kernelnewbies" to ecartis@nl.linux.org
> Please read the FAQ at http://kernelnewbies.org/FAQ
>
>

Re: How to fix warning 'control reaches end of non-void function'

[Johannes Weiner]

Hi,

Alexander Beregalov <a.beregalov@gmail.com> writes:

> Hi
>
> What is it a right way to fix these warnings?
>
> smth function()
> {
> 	switch (var) {
> 	case one:
> 		return 1;
> 	default:
> 		BUG();
> 	}
> }
>
> warning: control reaches end of non-void function

Uhm, make sure there is always a value returned...?  You don't do that
when var is not one.

	Hannes

Re: How to fix warning 'control reaches end of non-void function'

[Marcin Slusarz]

On Tue, Jul 29, 2008 at 11:17:38PM +0400, Alexander Beregalov wrote:
> Hi
> 
> What is it a right way to fix these warnings?
> 
> smth function()
> {
> 	switch (var) {
> 	case one:
> 		return 1;
> 	default:
> 		BUG();
> 	}
> }
> 
> warning: control reaches end of non-void function

If BUG() would be normal function it could be marked as __attribute__((noreturn)).
But it's a macro defined differently depending on architecture and CONFIG_*.
Sometimes it calls panic() which is annotated properly and sometimes it calls
other code....

If you are brave, you could fix all implementations.

Marcin

Re: How to fix warning 'control reaches end of non-void function'

[Johannes Weiner]

Hi,

Marcin Slusarz <marcin.slusarz@gmail.com> writes:

> On Tue, Jul 29, 2008 at 11:17:38PM +0400, Alexander Beregalov wrote:
>> Hi
>> 
>> What is it a right way to fix these warnings?
>> 
>> smth function()
>> {
>> 	switch (var) {
>> 	case one:
>> 		return 1;
>> 	default:
>> 		BUG();
>> 	}
>> }
>> 
>> warning: control reaches end of non-void function
>
> If BUG() would be normal function it could be marked as __attribute__((noreturn)).
> But it's a macro defined differently depending on architecture and CONFIG_*.
> Sometimes it calls panic() which is annotated properly and sometimes it calls
> other code....
>
> If you are brave, you could fix all implementations.

And sometimes it calls nothing at all (!CONFIG_BUG).  So even if you
mark the thing that _does_ something as noreturn, it will still emit
warnings then on !CONFIG_BUG.

	Hannes

Re: How to fix warning 'control reaches end of non-void function'

[Rene Herman]

On 29-07-08 21:17, Alexander Beregalov wrote:

> What is it a right way to fix these warnings?
> 
> smth function()
> {
> 	switch (var) {
> 	case one:
> 		return 1;
> 	default:
> 		BUG();
> 	}
> }
> 
> warning: control reaches end of non-void function

BUG() may actually do nothing so I'd just reorder things around as

int function(void)
{
	switch (var) {
	case 1:
		break;
	default:
		BUG();
	}
	return 1;
}

Rene.

Re: How to fix warning 'control reaches end of non-void function'

[Jacob]

If we haven't beat this to death yet, the right way to fix these warnings
is to return a "smth" data type... which most people have commented on

// original version
smth function()
{
       switch (var) {
       case one:
               return 1;
       default:
               BUG();
       }
}

// modified version
smth function()
{
       smth ret = 0;
       switch (var) {   // unless this is a global var is undefined
       case one:
               ret = 1;  // 1 was not a 'smth' type so we should use
the function's return type or face compiler errors/warnings
               break;   // unless you want hard to find bugs always
include a break;
       default:
               BUG(); // if this is a macro does it return a value?
               ret = 0;
               break;
       }
       return ret;  // since we are returning a 'smth' type we do not
let control reach the end of this non-void funtion
}


Where does this function get used, and what is it used for?

Jacob



On Tue, Jul 29, 2008 at 3:38 PM, Rene Herman <rene.herman@keyaccess.nl> wrote:
> On 29-07-08 21:17, Alexander Beregalov wrote:
>
>> What is it a right way to fix these warnings?
>>
>> smth function()
>> {
>>        switch (var) {
>>        case one:
>>                return 1;
>>        default:
>>                BUG();
>>        }
>> }
>>
>> warning: control reaches end of non-void function
>
> BUG() may actually do nothing so I'd just reorder things around as
>
> int function(void)
> {
>        switch (var) {
>        case 1:
>                break;
>        default:
>                BUG();
>        }
>        return 1;
> }
>
> Rene.
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors"
> in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: How to fix warning 'control reaches end of non-void function'

[Alexander Beregalov]

2008/7/30 Jacob <jacobchappelle@gmail.com>:
> If we haven't beat this to death yet, the right way to fix these warnings
> is to return a "smth" data type... which most people have commented on
>
> // original version
> smth function()
> {
>       switch (var) {
>       case one:
>               return 1;
>       default:
>               BUG();
>       }
> }
>
> // modified version
> smth function()
> {
>       smth ret = 0;
>       switch (var) {   // unless this is a global var is undefined
>       case one:
>               ret = 1;  // 1 was not a 'smth' type so we should use
> the function's return type or face compiler errors/warnings
>               break;   // unless you want hard to find bugs always
> include a break;
>       default:
>               BUG(); // if this is a macro does it return a value?
>               ret = 0;
>               break;
>       }
>       return ret;  // since we are returning a 'smth' type we do not
> let control reach the end of this non-void funtion
> }

Looks good, but how can I know which value should it return in that case?
>
>
> Where does this function get used, and what is it used for?
Ok, some examples

kernel/cpuset.c:

static u64 cpuset_read_u64(struct cgroup *cont, struct cftype *cft)
{
        struct cpuset *cs = cgroup_cs(cont);
        cpuset_filetype_t type = cft->private;
        switch (type) {
        case FILE_CPU_EXCLUSIVE:
                return is_cpu_exclusive(cs);
        case FILE_MEM_EXCLUSIVE:
                return is_mem_exclusive(cs);
        case FILE_MEM_HARDWALL:
                return is_mem_hardwall(cs);
        case FILE_SCHED_LOAD_BALANCE:
                return is_sched_load_balance(cs);
        case FILE_MEMORY_MIGRATE:
                return is_memory_migrate(cs);
        case FILE_MEMORY_PRESSURE_ENABLED:
                return cpuset_memory_pressure_enabled;
        case FILE_MEMORY_PRESSURE:
                return fmeter_getrate(&cs->fmeter);
        case FILE_SPREAD_PAGE:
                return is_spread_page(cs);
        case FILE_SPREAD_SLAB:
                return is_spread_slab(cs);
        default:
                BUG();
        }
}

static s64 cpuset_read_s64(struct cgroup *cont, struct cftype *cft)
{
        struct cpuset *cs = cgroup_cs(cont);
        cpuset_filetype_t type = cft->private;
        switch (type) {
        case FILE_SCHED_RELAX_DOMAIN_LEVEL:
                return cs->relax_domain_level;
        default:
                BUG();
        }
}


mm/mempolicy.c:

/*
 * Depending on the memory policy provide a node from which to allocate the
 * next slab entry.
 * @policy must be protected by freeing by the caller.  If @policy is
 * the current task's mempolicy, this protection is implicit, as only the
 * task can change it's policy.  The system default policy requires no
 * such protection.
 */
unsigned slab_node(struct mempolicy *policy)
{
        if (!policy || policy->flags & MPOL_F_LOCAL)
                return numa_node_id();

        switch (policy->mode) {
        case MPOL_PREFERRED:
                /*
                 * handled MPOL_F_LOCAL above
                 */
                return policy->v.preferred_node;

        case MPOL_INTERLEAVE:
                return interleave_nodes(policy);

        case MPOL_BIND: {
                /*
                 * Follow bind policy behavior and start allocation at the
                 * first node.
                 */
                struct zonelist *zonelist;
                struct zone *zone;
                enum zone_type highest_zoneidx = gfp_zone(GFP_KERNEL);
                zonelist = &NODE_DATA(numa_node_id())->node_zonelists[0];
                (void)first_zones_zonelist(zonelist, highest_zoneidx,
                                                        &policy->v.nodes,
                                                        &zone);
                return zone->node;
        }

        default:
                BUG();
        }
}


drivers/net/sky2.c:

/* Chip internal frequency for clock calculations */
static u32 sky2_mhz(const struct sky2_hw *hw)
{
        switch (hw->chip_id) {
        case CHIP_ID_YUKON_EC:
        case CHIP_ID_YUKON_EC_U:
        case CHIP_ID_YUKON_EX:
        case CHIP_ID_YUKON_SUPR:
        case CHIP_ID_YUKON_UL_2:
                return 125;

        case CHIP_ID_YUKON_FE:
                return 100;

        case CHIP_ID_YUKON_FE_P:
                return 50;

        case CHIP_ID_YUKON_XL:
                return 156;

        default:
                BUG();
        }
}

Re: How to fix warning 'control reaches end of non-void function'

[Sandeep K Sinha]

On Wed, Jul 30, 2008 at 4:57 AM, Alexander Beregalov
<a.beregalov@gmail.com> wrote:
> 2008/7/30 Jacob <jacobchappelle@gmail.com>:
>> If we haven't beat this to death yet, the right way to fix these warnings
>> is to return a "smth" data type... which most people have commented on
>>
>> // original version
>> smth function()
>> {
>>       switch (var) {
>>       case one:
>>               return 1;
>>       default:
>>               BUG();
>>       }
>> }
>>
>> // modified version
>> smth function()
>> {
>>       smth ret = 0;
>>       switch (var) {   // unless this is a global var is undefined
>>       case one:
>>               ret = 1;  // 1 was not a 'smth' type so we should use
>> the function's return type or face compiler errors/warnings
>>               break;   // unless you want hard to find bugs always
>> include a break;
>>       default:
>>               BUG(); // if this is a macro does it return a value?
>>               ret = 0;
>>               break;
>>       }
>>       return ret;  // since we are returning a 'smth' type we do not
>> let control reach the end of this non-void funtion
>> }
>
> Looks good, but how can I know which value should it return in that case?
>>
Well, If we look at the standard way and we wish to be philosophically
right, if the return type is non void, we must return a value. Now,
its the choice of the developer to handle it, how !
>>
>> Where does this function get used, and what is it used for?
> Ok, some examples
>
> kernel/cpuset.c:
>
> static u64 cpuset_read_u64(struct cgroup *cont, struct cftype *cft)
> {
>        struct cpuset *cs = cgroup_cs(cont);
>        cpuset_filetype_t type = cft->private;
>        switch (type) {
>        case FILE_CPU_EXCLUSIVE:
>                return is_cpu_exclusive(cs);
>        case FILE_MEM_EXCLUSIVE:
>                return is_mem_exclusive(cs);
>        case FILE_MEM_HARDWALL:
>                return is_mem_hardwall(cs);
>        case FILE_SCHED_LOAD_BALANCE:
>                return is_sched_load_balance(cs);
>        case FILE_MEMORY_MIGRATE:
>                return is_memory_migrate(cs);
>        case FILE_MEMORY_PRESSURE_ENABLED:
>                return cpuset_memory_pressure_enabled;
>        case FILE_MEMORY_PRESSURE:
>                return fmeter_getrate(&cs->fmeter);
>        case FILE_SPREAD_PAGE:
>                return is_spread_page(cs);
>        case FILE_SPREAD_SLAB:
>                return is_spread_slab(cs);
>        default:
>                BUG();
>        }
> }
>
> static s64 cpuset_read_s64(struct cgroup *cont, struct cftype *cft)
> {
>        struct cpuset *cs = cgroup_cs(cont);
>        cpuset_filetype_t type = cft->private;
>        switch (type) {
>        case FILE_SCHED_RELAX_DOMAIN_LEVEL:
>                return cs->relax_domain_level;
>        default:
>                BUG();
>        }
> }
>
>
> mm/mempolicy.c:
>
> /*
>  * Depending on the memory policy provide a node from which to allocate the
>  * next slab entry.
>  * @policy must be protected by freeing by the caller.  If @policy is
>  * the current task's mempolicy, this protection is implicit, as only the
>  * task can change it's policy.  The system default policy requires no
>  * such protection.
>  */
> unsigned slab_node(struct mempolicy *policy)
> {
>        if (!policy || policy->flags & MPOL_F_LOCAL)
>                return numa_node_id();
>
>        switch (policy->mode) {
>        case MPOL_PREFERRED:
>                /*
>                 * handled MPOL_F_LOCAL above
>                 */
>                return policy->v.preferred_node;
>
>        case MPOL_INTERLEAVE:
>                return interleave_nodes(policy);
>
>        case MPOL_BIND: {
>                /*
>                 * Follow bind policy behavior and start allocation at the
>                 * first node.
>                 */
>                struct zonelist *zonelist;
>                struct zone *zone;
>                enum zone_type highest_zoneidx = gfp_zone(GFP_KERNEL);
>                zonelist = &NODE_DATA(numa_node_id())->node_zonelists[0];
>                (void)first_zones_zonelist(zonelist, highest_zoneidx,
>                                                        &policy->v.nodes,
>                                                        &zone);
>                return zone->node;
>        }
>
>        default:
>                BUG();
>        }
> }
>
>
> drivers/net/sky2.c:
>
> /* Chip internal frequency for clock calculations */
> static u32 sky2_mhz(const struct sky2_hw *hw)
> {
>        switch (hw->chip_id) {
>        case CHIP_ID_YUKON_EC:
>        case CHIP_ID_YUKON_EC_U:
>        case CHIP_ID_YUKON_EX:
>        case CHIP_ID_YUKON_SUPR:
>        case CHIP_ID_YUKON_UL_2:
>                return 125;
>
>        case CHIP_ID_YUKON_FE:
>                return 100;
>
>        case CHIP_ID_YUKON_FE_P:
>                return 50;
>
>        case CHIP_ID_YUKON_XL:
>                return 156;
>
>        default:
>                BUG();
>        }
> }
>
> --
> To unsubscribe from this list: send an email with
> "unsubscribe kernelnewbies" to ecartis@nl.linux.org
> Please read the FAQ at http://kernelnewbies.org/FAQ
>
>



-- 
Regards,
Sandeep.






"To learn is to change. Education is a process that changes the learner."

Re: How to fix warning 'control reaches end of non-void function'

[Hugo Vasconcelos Saldanha]

On Tue, Jul 29, 2008 at 8:27 PM, Alexander Beregalov
<a.beregalov@gmail.com> wrote:
>
> Looks good, but how can I know which value should it return in that case?
>

I think you should specify an error code to return, letting the caller
decide what to do with it.

--
Hugo

Re: How to fix warning 'control reaches end of non-void function'

[walter harms]

Hugo Vasconcelos Saldanha wrote:
> On Tue, Jul 29, 2008 at 8:27 PM, Alexander Beregalov
> <a.beregalov@gmail.com> wrote:
>> Looks good, but how can I know which value should it return in that case?
>>
> 
> I think you should specify an error code to return, letting the caller
> decide what to do with it.
> 

You need to know
* can it be fixed ? the kernel/cpuset.c clearly shows NO
i have an (wrong?) argument that can not be fixed.

* is it save to leave untouched ?
(no idea in that case).
if yes document there is a unknown argument syslog()
or panic()

in some cases it may even be useful to make a function void().

re,
 wh