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 for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <486bddb6aad14d05a3fb2d876d0d9d0d@manjaro.org>
Date: Wed, 21 Aug 2024 11:09:03 +0200
From: Dragan Simic <dsimic@...jaro.org>
To: Heiko Stübner <heiko@...ech.de>
Cc: linux-rockchip@...ts.infradead.org, linux-phy@...ts.infradead.org,
 vkoul@...nel.org, kishon@...nel.org, linux-arm-kernel@...ts.infradead.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 3/3] phy: phy-rockchip-inno-usb2: Improve error
 handling while probing

On 2024-08-21 10:44, Heiko Stübner wrote:
> Am Mittwoch, 21. August 2024, 09:37:55 CEST schrieb Dragan Simic:
>> Improve error handling in the probe path by using function 
>> dev_err_probe()
>> where appropriate, and by no longer using it rather pointlessly in one 
>> place
>> that actually produces a single, hardcoded error code.
>> 
>> Signed-off-by: Dragan Simic <dsimic@...jaro.org>
> 
>> @@ -1375,8 +1372,10 @@ static int rockchip_usb2phy_probe(struct 
>> platform_device *pdev)
>>  	rphy->irq = platform_get_irq_optional(pdev, 0);
>>  	platform_set_drvdata(pdev, rphy);
>> 
>> -	if (!phy_cfgs)
>> -		return dev_err_probe(dev, -EINVAL, "phy configs are not 
>> assigned!\n");
>> +	if (!phy_cfgs) {
>> +		dev_err(dev, "phy configs are not assigned\n");
>> +		return -EINVAL;
>> +	}
>> 
>>  	ret = rockchip_usb2phy_extcon_register(rphy);
>>  	if (ret)
> 
> I really don't understand the rationale here. Using dev_err_probe here
> is just fine and with that change you just introduce more lines of code
> for exactly the same functionality?

As we know, dev_err_probe() decides how to log the received error 
message
based on the error code it receives, but in this case the error code is
hardcoded as -EINVAL.  Thus, in this case it isn't about keeping the LoC
count a bit lower, but about using dev_err() where the resulting outcome
of error logging is aleady known, and where logging the error code 
actually
isn't helpful, because it's hardcoded and the logged error message 
already
tells everything about the error condition.

In other words, it's about being as precise as possible when deciding 
between
dev_err() and dev_err_probe(), in both directions.  I hope it makes 
sense.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ