Skip to contentSkip to conversation

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

DISCUSSION Alexander Beregalov 9 participants 11 messages Jul 29, 2008 – Jul 30, 2008 Kernel janitors

lore
Share & tools
lore
#0 lore raw

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!

#1 lore raw

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.

Quoted 25 lines · 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!

--
To unsubscribe from this list: send an email with "unsubscribe kernelnewbies" to [email protected] Please read the FAQ at http://kernelnewbies.org/FAQ

#2 lore raw
In reply to Alexander Beregalov

Hi,

Quoted 14 lines · Alexander Beregalov

Hi

What is it a right way to fix these warnings?

smth function()

{
	switch (var) {
	case one:
		return 1;
	default:
		BUG();
	}
}
Quoted 1 line

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

#3 lore raw
In reply to Alexander Beregalov
Quoted 14 lines · Alexander Beregalov

Hi

What is it a right way to fix these warnings?

smth function()

{
	switch (var) {
	case one:
		return 1;
	default:
		BUG();
	}
}
Quoted 1 line

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

#4 lore raw
In reply to Alexander Beregalov

Hi,

Quoted 15 lines · Marcin Slusarz
Quoted 14 lines · Alexander Beregalov

Hi

What is it a right way to fix these warnings?

smth function()

{
	switch (var) {
	case one:
		return 1;
	default:
		BUG();
	}
}
Quoted 8 lines
Quoted 1 line

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

#5 lore raw
In reply to Alexander Beregalov
Quoted 12 lines · Alexander Beregalov

What is it a right way to fix these warnings?

smth function()

{
	switch (var) {
	case one:
		return 1;
	default:
		BUG();
	}
}
Quoted 1 line

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.

#6 lore raw
In reply to Alexander Beregalov

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

Quoted 36 lines · Rene Herman
Quoted 12 lines · Alexander Beregalov

What is it a right way to fix these warnings?

smth function()

{
       switch (var) {
       case one:
               return 1;
       default:
               BUG();
       }
}
Quoted 1 line

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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#7 lore raw
In reply to Alexander Beregalov

2008/7/30 Jacob <[email protected]>:

Quoted 30 lines

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
Quoted 2 lines

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?

Quoted 3 lines

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();
        }
}
#8 lore raw
In reply to Alexander Beregalov
Quoted 34 lines · Alexander Beregalov

2008/7/30 Jacob <[email protected]>:

Quoted 30 lines

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
Quoted 2 lines

let control reach the end of this non-void funtion }

Quoted 2 lines

Looks good, but how can I know which value should it return in that case?

Quoted 1 line

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 !

Quoted 126 lines
Quoted 2 lines

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 [email protected] Please read the FAQ at http://kernelnewbies.org/FAQ

Signature · 9 lines
Regards,
Sandeep.






"To learn is to change. Education is a process that changes the learner."
#10 lore raw
In reply to Alexander Beregalov
Quoted 8 lines · Hugo Vasconcelos Saldanha
Quoted 2 lines · Alexander Beregalov

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