Messages in this thread | | | Date | Tue, 29 Nov 2022 08:19:56 +0300 | From | Dan Carpenter <> | Subject | Re: drivers/cpuidle/cpuidle-riscv-sbi.c:506 sbi_genpd_probe() warn: missing error code 'ret' |
| |
On Thu, Nov 24, 2022 at 01:35:33PM +0000, Conor Dooley wrote: > Hey Anup, > > On Tue, Nov 22, 2022 at 01:43:38PM +0530, Anup Patel wrote: > > On Tue, Nov 22, 2022 at 12:41 PM Dan Carpenter <error27@gmail.com> wrote: > > > > > > tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master > > > head: eb7081409f94a9a8608593d0fb63a1aa3d6f95d8 > > > commit: f81f7861ee2aaa6f652f18e8f622547bdd379724 cpuidle: riscv: support non-SMP config > > > date: 7 months ago > > > config: riscv-randconfig-m031-20221121 > > > compiler: riscv64-linux-gcc (GCC) 12.1.0 > > > > > > If you fix the issue, kindly add following tag where applicable > > > | Reported-by: kernel test robot <lkp@intel.com> > > > | Reported-by: Dan Carpenter <error27@gmail.com> > > > > > > smatch warnings: > > > drivers/cpuidle/cpuidle-riscv-sbi.c:506 sbi_genpd_probe() warn: missing error code 'ret' > > > > > > vim +/ret +506 drivers/cpuidle/cpuidle-riscv-sbi.c > > > > > > 6abf32f1d9c500 Anup Patel 2022-02-10 481 static int sbi_genpd_probe(struct device_node *np) > > > 6abf32f1d9c500 Anup Patel 2022-02-10 482 { > > > 6abf32f1d9c500 Anup Patel 2022-02-10 483 struct device_node *node; > > > 6abf32f1d9c500 Anup Patel 2022-02-10 484 int ret = 0, pd_count = 0; > > > 6abf32f1d9c500 Anup Patel 2022-02-10 485 > > > 6abf32f1d9c500 Anup Patel 2022-02-10 486 if (!np) > > > 6abf32f1d9c500 Anup Patel 2022-02-10 487 return -ENODEV; > > > 6abf32f1d9c500 Anup Patel 2022-02-10 488 > > > 6abf32f1d9c500 Anup Patel 2022-02-10 489 /* > > > 6abf32f1d9c500 Anup Patel 2022-02-10 490 * Parse child nodes for the "#power-domain-cells" property and > > > 6abf32f1d9c500 Anup Patel 2022-02-10 491 * initialize a genpd/genpd-of-provider pair when it's found. > > > 6abf32f1d9c500 Anup Patel 2022-02-10 492 */ > > > 6abf32f1d9c500 Anup Patel 2022-02-10 493 for_each_child_of_node(np, node) { > > > 6abf32f1d9c500 Anup Patel 2022-02-10 494 if (!of_find_property(node, "#power-domain-cells", NULL)) > > > 6abf32f1d9c500 Anup Patel 2022-02-10 495 continue; > > > 6abf32f1d9c500 Anup Patel 2022-02-10 496 > > > 6abf32f1d9c500 Anup Patel 2022-02-10 497 ret = sbi_pd_init(node); > > > 6abf32f1d9c500 Anup Patel 2022-02-10 498 if (ret) > > > 6abf32f1d9c500 Anup Patel 2022-02-10 499 goto put_node; > > > 6abf32f1d9c500 Anup Patel 2022-02-10 500 > > > 6abf32f1d9c500 Anup Patel 2022-02-10 501 pd_count++; > > > 6abf32f1d9c500 Anup Patel 2022-02-10 502 } > > > 6abf32f1d9c500 Anup Patel 2022-02-10 503 > > > 6abf32f1d9c500 Anup Patel 2022-02-10 504 /* Bail out if not using the hierarchical CPU topology. */ > > > 6abf32f1d9c500 Anup Patel 2022-02-10 505 if (!pd_count) > > > 6abf32f1d9c500 Anup Patel 2022-02-10 @506 goto no_pd; > > > > > > Error code? > > > > Yes, we intentionally "return 0" when there are no > > generic power-domains defined for the CPUs, the > > sbi_cpuidle_probe() continue further and try traditional > > DT cpuidle states. > > Happened upon this when looking for our other cpuidle conversation on > lore earlier, would it not make more sense from a readability PoV to > just return zero here?
I am always in favor of direct returns over a do nothing return because of the ambiguity about error codes. Also I just published a new Smatch check where "return ret;" and "return 0;" are equivalent.
ret = frob(); if (ret) return ret;
if (something else) return ret;
I have a different unpublished check for:
ret = frob(); if (!ret) return ret;
The bug I'm looking for here is that once or twice a year the ! character is unintentional.
regards, dan carpenter
| |