Messages in this thread | | | From | <> | Subject | Re: [PATCH v4] powerpc:85xx: Add missing of_node_put() in sgy_cst1000 | Date | Fri, 17 Jun 2022 08:27:03 +0000 |
| |
On 17/06/2022 09:17, Liang He wrote: > > > > At 2022-06-17 14:53:13, "Christophe Leroy" <christophe.leroy@csgroup.eu> wrote: >> >> >> Le 17/06/2022 à 08:45, Liang He a écrit : >>> >>> >>> >>> At 2022-06-17 14:28:56, "Christophe Leroy" <christophe.leroy@csgroup.eu> wrote: >>>> >>>> >>>> Le 17/06/2022 à 08:08, Liang He a écrit : >>>>> In gpio_halt_probe(), of_find_matching_node() will return a node >>>>> pointer with refcount incremented. We should use of_node_put() in >>>>> fail path or when it is not used anymore. >>>>> >>>>> Signed-off-by: Liang He <windhl@126.com> >>>>> --- >>>>> changelog: >>>>> v4: reuse exist 'err' and use a simple code style, advised by CJ >>>>> v3: use local 'child_node' advised by Michael. >>>>> v2: use goto-label patch style advised by Christophe Leroy. >>>>> v1: add of_node_put() before each exit. >>>>> >>>>> arch/powerpc/platforms/85xx/sgy_cts1000.c | 35 ++++++++++++++--------- >>>>> 1 file changed, 22 insertions(+), 13 deletions(-) >>>>> >>>>> diff --git a/arch/powerpc/platforms/85xx/sgy_cts1000.c b/arch/powerpc/platforms/85xx/sgy_cts1000.c >>>>> index 98ae64075193..e4588943fe7e 100644 >>>>> --- a/arch/powerpc/platforms/85xx/sgy_cts1000.c >>>>> +++ b/arch/powerpc/platforms/85xx/sgy_cts1000.c >>>>> @@ -71,6 +71,7 @@ static int gpio_halt_probe(struct platform_device *pdev) >>>>> { >>>>> enum of_gpio_flags flags; >>>>> struct device_node *node = pdev->dev.of_node; >>>>> + struct device_node *child_node; >>>>> int gpio, err, irq; >>>>> int trigger; >>>>> >>>>> @@ -78,26 +79,29 @@ static int gpio_halt_probe(struct platform_device *pdev) >>>>> return -ENODEV; >>>>> >>>>> /* If there's no matching child, this isn't really an error */ >>>>> - halt_node = of_find_matching_node(node, child_match); >>>>> - if (!halt_node) >>>>> + child_node = of_find_matching_node(node, child_match); >>>>> + if (!child_node) >>>>> return 0; >>>>> >>>>> /* Technically we could just read the first one, but punish >>>>> * DT writers for invalid form. */ >>>>> - if (of_gpio_count(halt_node) != 1) >>>>> - return -EINVAL; >>>>> + if (of_gpio_count(child_node) != 1) { >>>>> + err = -EINVAL; >>>>> + goto err_put; >>>>> + } >>>>> >>>>> /* Get the gpio number relative to the dynamic base. */ >>>>> - gpio = of_get_gpio_flags(halt_node, 0, &flags); >>>>> - if (!gpio_is_valid(gpio)) >>>>> - return -EINVAL; >>>>> + gpio = of_get_gpio_flags(child_node, 0, &flags); >>>>> + if (!gpio_is_valid(gpio)) { >>>>> + err = -EINVAL; >>>>> + gotot err_put; >>>> >>>> Did you test the build ? >>> >>> Sorry for this fault. >>> >>> In fact, I am still finding an efficient way to building different arch source code as I only have x86-64. >>> >>> Now I am try using QEMU. >>> >>> Anyway, sorry for this fault. >> >> You can find cross compilers for most architectures for x86-64 here : >> https://mirrors.edge.kernel.org/pub/tools/crosstool/ >> >> Christophe > > Hi, Christophe and Conor. > > Sorry to trouble you again. > > Now I only know how to quickly identify the refcounting bugs, but I cannot efficiently give a build test. > > For example, I use the cross compilers 'powerpc-linux-gnu-gcc' to compile 'arch/powerpc/platforms/85xx/sgy_cts1000.c' with -fsyntax-only flag. > But I meet too many header file missing errors. Even if I add some 'include' pathes, e.g., ./arch/powerpc/include, ./include, > there are still too many other errors. > > So if there is any efficient way to check my patch code to avoid 'gotot' error again.
idk anything about powerpc, but what I find is a nice way to get a compiler for an arch I don't use is to search on lore.kernel.org for a 0day robot build error since it gives instructions for building on that arch. For example: https://lore.kernel.org/linuxppc-dev/202206060910.rYNTFqdI-lkp@intel.com/
In this case, your bug seems obvious? You typed "gotot" instead of "goto".
Hope that helps, Conor.
> > Thanks again, Christophe and Conor. > > Liang
| |