From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1753195AbdJ3VIw (ORCPT <rfc822;w@1wt.eu>);
        Mon, 30 Oct 2017 17:08:52 -0400
Received: from mail-lf0-f65.google.com ([209.85.215.65]:54926 "EHLO
        mail-lf0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1752466AbdJ3VIt (ORCPT
        <rfc822;linux-kernel@vger.kernel.org>);
        Mon, 30 Oct 2017 17:08:49 -0400
X-Google-Smtp-Source: ABhQp+TsbucYdwSuhAeQt5LZhdDkXV2SZJUqmMaT9GXvPNqQn970sSjaYRSsQ5JjyYR5IGqqD8M10w==
Subject: Re: [PATCH] ravb: Use common error handling code in ravb_probe()
To: Geert Uytterhoeven <geert@linux-m68k.org>,
        SF Markus Elfring <elfring@users.sourceforge.net>
Cc: "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        Linux-Renesas <linux-renesas-soc@vger.kernel.org>,
        Dan Carpenter <dan.carpenter@oracle.com>,
        "David S. Miller" <davem@davemloft.net>,
        Eugeniu Rosca <erosca@de.adit-jv.com>,
        Kazuya Mizuguchi <kazuya.mizuguchi.ks@renesas.com>,
        Masaru Nagai <masaru.nagai.vx@renesas.com>,
        =?UTF-8?Q?Niklas_S=c3=b6derlund?= 
        <niklas.soderlund+renesas@ragnatech.se>,
        Simon Horman <horms+renesas@verge.net.au>,
        Yuval Shaia <yuval.shaia@oracle.com>,
        LKML <linux-kernel@vger.kernel.org>,
        "kernel-janitors@vger.kernel.org" <kernel-janitors@vger.kernel.org>
References: <2839c3c2-0116-7549-6ff4-a49eb0a52298@users.sourceforge.net>
 <CAMuHMdXEWpryexcrN7OYcsGfyE3Y_-7Ej2CfgvYgUbZFTTHEWg@mail.gmail.com>
From: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Organization: Cogent Embedded
Message-ID: <161f4ef9-89b7-bfad-89e7-06bdeeb9ac31@cogentembedded.com>
Date: Tue, 31 Oct 2017 00:08:44 +0300
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
 Thunderbird/52.2.1
MIME-Version: 1.0
In-Reply-To: <CAMuHMdXEWpryexcrN7OYcsGfyE3Y_-7Ej2CfgvYgUbZFTTHEWg@mail.gmail.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-MW
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 10/29/2017 02:00 PM, Geert Uytterhoeven wrote:

>> From: Markus Elfring <elfring@users.sourceforge.net>
>> Date: Sat, 28 Oct 2017 19:10:08 +0200
>>
>> Add a jump target so that a bit of exception handling can be better reused
>> at the end of this function.
>>
>> This issue was detected by using the Coccinelle software.
>>
>> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
>> ---
>>   drivers/net/ethernet/renesas/ravb_main.c | 32 ++++++++++++++++----------------
>>   1 file changed, 16 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
>> index a8822a756e08..62dbdf7de6cd 100644
>> --- a/drivers/net/ethernet/renesas/ravb_main.c
>> +++ b/drivers/net/ethernet/renesas/ravb_main.c
>> @@ -2069,10 +2069,9 @@ static int ravb_probe(struct platform_device *pdev)
>>                  irq = platform_get_irq_byname(pdev, "ch22");
>>          else
>>                  irq = platform_get_irq(pdev, 0);
>> -       if (irq < 0) {
>> -               error = irq;
>> -               goto out_release;
>> -       }
>> +       if (irq < 0)
>> +               goto failure_indication;
> 
> IMHO, it's really confusing that "irq" contains the error code, not "error".

    I think it would have been equally confusing if 'error' was assigned to 
'ndev->irq', etc. It's just the duality of the result of these functions that 
makes them confusing...

> Especially when jumping to a meaningless label named "failure_indication"
> ("irq_failure" would be more intuitive).

    Yeah, the label sucks. :-)

> So I prefer the original code, regardless of the label name.

    On the 2nd thought, the patch can be fixed up and then merged.

> Gr{oetje,eeting}s,
> 
>                          Geert

MBR, Sergei