lkml.org 
[lkml]   [2022]   [Jun]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [PATCH v4] powerpc:85xx: Add missing of_node_put() in sgy_cst1000
    Date
    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
    \
     
     \ /
      Last update: 2022-06-17 10:28    [W:6.763 / U:1.464 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site