[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <28ab70eb-8b75-6a56-1406-859a51ea4684@huawei.com>
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;
> .
>
Powered by blists - more mailing lists