linux-kernel - Re: [RFC/PATCH 1/5] mtd: ubi: Read disturb infrastructure

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: <5428E42A.6090206@codeaurora.org>
Date:	Mon, 29 Sep 2014 07:46:34 +0300
From:	Tanya Brokhman <tlinder@...eaurora.org>
To:	Jeremiah Mahler <jmmahler@...il.com>, linux-kernel@...r.kernel.org
Subject: Re: [RFC/PATCH 1/5] mtd: ubi: Read disturb infrastructure

Hi Jeremiah,

On 9/28/2014 9:13 PM, Jeremiah Mahler wrote:
> Tanya,
>
> On Sun, Sep 28, 2014 at 09:37:00AM +0300, Tanya Brokhman wrote:
>> The need for performing read disturb is determined according to new
>> statistics collected per eraseblock:
>> - read counter: incremented at each read operation
>>                  reset at each erase
>> - last erase time stamp: updated at each erase
>>
>> This patch adds the infrastructure for the above statistics
>>
>> Signed-off-by: Tanya Brokhman <tlinder@...eaurora.org>
>> ---
> ...
>> @@ -385,6 +402,38 @@ static ssize_t dev_attribute_show(struct device *dev,
>>   	return ret;
>>   }
>>
>> +static ssize_t dev_attribute_store(struct device *dev,
>> +			   struct device_attribute *attr,
>> +			   const char *buf, size_t count)
>> +{
>> +	int value;
>> +	struct ubi_device *ubi;
>> +
>> +	ubi = container_of(dev, struct ubi_device, dev);
>> +	ubi = ubi_get_device(ubi->ubi_num);
>> +	if (!ubi)
>> +		return -ENODEV;
>> +
>> +	if (kstrtos32(buf, 10, &value))
>> +		return -EINVAL;
>> +	/* Consider triggering full scan if threshods change */
>> +	else if (attr == &dev_dt_threshold) {
>> +		if (value < UBI_MAX_DT_THRESHOLD)
>> +			ubi->dt_threshold = value;
>> +		else
>> +			pr_err("Max supported threshold value is %d",
>> +				   UBI_MAX_DT_THRESHOLD);
>> +	} else if (attr == &dev_rd_threshold) {
>> +		if (value < UBI_MAX_READCOUNTER)
>> +			ubi->rd_threshold = value;
>> +		else
>> +			pr_err("Max supported threshold value is %d",
>> +				   UBI_MAX_READCOUNTER);
>> +	}
>> +
>> +	return count;
>> +}
>> +
> ...
>
> One small style nit.  As per Documentation/CodingStyle [line 169] if
> one branch in a conditional uses braces then all branches should use
> braces.
>

I'm sorry but I understand it differently. From CodingStyle:
"This does not apply if only one branch of a conditional statement is a 
single
statement; in the latter case use braces in both branches:

if (condition) {
	do_this();
	do_that();
} else {
	otherwise();
}"
According to my understanding this doesn't mean {} should be added in 
case of an if statement inside an if statement. So the above code seems 
to be complaint with the coding style.
Please correct me if I'm misunderstanding something.

Thanks,
- Tanya Brokhman

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, 
hosted by The Linux Foundation
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/