linux-kernel - Re: [PATCH] bus: ti-sysc: Remove if-block in sysc_check

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]

Message-ID: <a1f56fcc-2207-fa32-83bc-cd219c2b893c@gmail.com>
Date:   Tue, 13 Aug 2019 10:05:41 +0530
From:   Nishka Dasgupta <nishkadg.linux@...il.com>
To:     Roger Quadros <rogerq@...com>, tony@...mide.com,
        linux-kernel@...r.kernel.org, "Kristo, Tero" <t-kristo@...com>
Subject: Re: [PATCH] bus: ti-sysc: Remove if-block in sysc_check_children()

On 08/08/19 7:25 PM, Roger Quadros wrote:
> Nishka,
> 
> On 08/08/2019 10:40, Nishka Dasgupta wrote:
>> In function sysc_check_children, there is an if-statement checking
>> whether the value returned by function sysc_check_one_child is non-zero.
>> However, sysc_check_one_child always returns 0, and hence this check is
>> not needed. Hence remove this if-block.
>>
>> Signed-off-by: Nishka Dasgupta <nishkadg.linux@...il.com>
>> ---
>>   drivers/bus/ti-sysc.c | 2 --
>>   1 file changed, 2 deletions(-)
>>
>> diff --git a/drivers/bus/ti-sysc.c b/drivers/bus/ti-sysc.c
>> index e6deabd8305d..bc8082ae7cb5 100644
>> --- a/drivers/bus/ti-sysc.c
>> +++ b/drivers/bus/ti-sysc.c
>> @@ -637,8 +637,6 @@ static int sysc_check_children(struct sysc *ddata)
>>   
>>   	for_each_child_of_node(ddata->dev->of_node, child) {
>>   		error = sysc_check_one_child(ddata, child);
>> -		if (error)
>> -			return error;
> 
> We cannot assume that sysc_check_one_child() will never return error in the future.
> If it can never return an error then why does it have an int return type?

I'm not sure why it has an int return type, unfortunately. This is the 
function in its entirety:

static int sysc_check_one_child(struct sysc *ddata,
				struct device_node *np)
{
	const char *name;

	name = of_get_property(np, "ti,hwmods", NULL);
	if (name)
		dev_warn(ddata->dev, "really a child ti,hwmods property?");

	sysc_check_quirk_stdout(ddata, np);
	sysc_parse_dts_quirks(ddata, np, true);

	return 0;
}

I'm not sure how to understand this function. Do dev_warn() or 
sysc_check_quirk_stdout() or sysc_parse_dts_quirks() cause a non-zero 
return from sysc_check_one_child()? Should I drop my patch?

Thanking you,
Nishka
> 
>>   	}
>>   
>>   	return 0;
>>
> 
> cheers,
> -roger
>