linux-kernel - Re: [PATCH 2/3] coresight: ultrasoc-smb: simplify the code for check to

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:   Fri, 20 Oct 2023 10:33:42 +0800
From:   hejunhao <hejunhao3@...wei.com>
To:     Jonathan Cameron <Jonathan.Cameron@...wei.com>,
        <linuxarm@...wei.com>
CC:     <suzuki.poulose@....com>, <james.clark@....com>,
        <coresight@...ts.linaro.org>,
        <linux-arm-kernel@...ts.infradead.org>,
        <linux-kernel@...r.kernel.org>, <yangyicong@...wei.com>,
        <prime.zeng@...ilicon.com>
Subject: Re: [PATCH 2/3] coresight: ultrasoc-smb: simplify the code for check
 to_copy valid


On 2023/10/19 21:35, Jonathan Cameron wrote:
> On Thu, 12 Oct 2023 17:47:05 +0800
> Junhao He <hejunhao3@...wei.com> wrote:
>
>> We only need to check once when before using the to_copy variable
>> to simplify the code.
>>
>> Signed-off-by: Junhao He <hejunhao3@...wei.com>
> I'm not convinced this one is an improvement. Sometimes it's easier to just
> see the individual conditions checked even if we could combine them.
> It's easy to understand we don't copy data if:
> a) We ask for 0 data.
> b) There is 0 data
>
> Less easy to establish that with the extra wrap around code in there
> (even though that has no impact on to_copy if it is 0)
>
> Jonathan
>

Thanks, I will drop this patch.

Best regards,
Junhao.

>> ---
>>   drivers/hwtracing/coresight/ultrasoc-smb.c | 13 ++++---------
>>   1 file changed, 4 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/hwtracing/coresight/ultrasoc-smb.c b/drivers/hwtracing/coresight/ultrasoc-smb.c
>> index b08a619d1116..e78edc3480ce 100644
>> --- a/drivers/hwtracing/coresight/ultrasoc-smb.c
>> +++ b/drivers/hwtracing/coresight/ultrasoc-smb.c
>> @@ -127,20 +127,15 @@ static ssize_t smb_read(struct file *file, char __user *data, size_t len,
>>   					struct smb_drv_data, miscdev);
>>   	struct smb_data_buffer *sdb = &drvdata->sdb;
>>   	struct device *dev = &drvdata->csdev->dev;
>> -	ssize_t to_copy = 0;
>> -
>> -	if (!len)
>> -		return 0;
>> -
>> -	if (!sdb->data_size)
>> -		return 0;
>> -
>> -	to_copy = min(sdb->data_size, len);
>> +	ssize_t to_copy = min(sdb->data_size, len);
>>   
>>   	/* Copy parts of trace data when read pointer wrap around SMB buffer */
>>   	if (sdb->buf_rdptr + to_copy > sdb->buf_size)
>>   		to_copy = sdb->buf_size - sdb->buf_rdptr;
>>   
>> +	if (!to_copy)
>> +		return 0;
>> +
>>   	if (copy_to_user(data, sdb->buf_base + sdb->buf_rdptr, to_copy)) {
>>   		dev_dbg(dev, "Failed to copy data to user\n");
>>   		return -EFAULT;
> .
>