linux-kernel - Re: [PATCH 2/2] staging: qlge: fix else after return or break

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sat, 27 Jun 2020 07:57:25 +0800
From:   Coiby Xu <coiby.xu@...il.com>
To:     Joe Perches <joe@...ches.com>
Cc:     devel@...verdev.osuosl.org, Manish Chopra <manishc@...vell.com>,
        "supporter:QLOGIC QLGE 10Gb ETHERNET DRIVER" 
        <GR-Linux-NIC-Dev@...vell.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "open list:QLOGIC QLGE 10Gb ETHERNET DRIVER" <netdev@...r.kernel.org>,
        open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/2] staging: qlge: fix else after return or break

On Thu, Jun 25, 2020 at 03:13:14PM -0700, Joe Perches wrote:
>On Fri, 2020-06-26 at 05:57 +0800, Coiby Xu wrote:
>> Remove unnecessary elses after return or break.
>
>unrelated trivia:
>
>> diff --git a/drivers/staging/qlge/qlge_dbg.c b/drivers/staging/qlge/qlge_dbg.c
>[]
>> @@ -1391,12 +1391,11 @@ static void ql_dump_cam_entries(struct ql_adapter *qdev)
>>  			pr_err("%s: Failed read of mac index register\n",
>>  			       __func__);
>>  			return;
>> -		} else {
>> -			if (value[0])
>> -				pr_err("%s: CAM index %d CAM Lookup Lower = 0x%.08x:%.08x, Output = 0x%.08x\n",
>> -				       qdev->ndev->name, i, value[1], value[0],
>> -				       value[2]);
>
>looks like all of these could use netdev_err
>
>				netdev_err(qdev, "etc...",
>					   i, value[1], value[0], value[2]);
>
>etc...

Should we also replace all pr_errs with netdev_err in
ql_dump_* functions? I'm not sure how we will use ql_dump_*. For example,
ql_dump_regs is not referred by any kernel source, so I guess it's for
the sole purpose of debugging the driver by the developer. But one
pr_err in ql_dump_routing_entries which is called by dl_dump_regs doesn't
prints out the device name whereas the other does,

> void ql_dump_routing_entries(struct ql_adapter *qdev)
> {
> 	int i;
> 	u32 value;
>
> 	i = ql_sem_spinlock(qdev, SEM_RT_IDX_MASK);
> 	if (i)
> 		return;
> 	for (i = 0; i < 16; i++) {
> 		value = 0;
> 		if (ql_get_routing_reg(qdev, i, &value)) {
> 			pr_err("%s: Failed read of routing index register\n",
> 			       __func__);
> 			break;
> 		}
> 		if (value)
> 			pr_err("%s: Routing Mask %d = 0x%.08x\n",
> 			       qdev->ndev->name, i, value);
> 	}

--
Best regards,
Coiby