Hi Andrzej,

> On 24 Sep 2015, at 09:34, Andrzej Hajda <a.ha...@samsung.com> wrote:
> 
> On 09/23/2015 12:21 PM, Anton Altaparmakov wrote:
>> Hi Andrzej,
>> 
>> Thanks for your patch.  It looks fine though I don't quite see the point of 
>> it to be honest.
>> 
>> It actually adds an additional function call (kmemdup() is not inline) just 
>> to save 1 line of source code in the driver and I don't think it improves 
>> readability or anything so why bother?  What does it gain?
> 
> kmemdup replaces combo (kmalloc + memdup) with one call.
> The patch follows quite common practice of abstracting out common patterns.

Sure it does I am just questioning the sanity of the practice...  (-;

Such changes reduce the size of the kernel binary by a few bytes at the cost of 
adding CPU cycles to the execution time.  How is that good thing?  Unless you 
are on an embedded system desperate for RAM throwing away CPU cycles on 
pointless abstractions makes no sense to me...

But as I said patch is fine.  Feel free to send it onto Andrew to get it into 
mainline.  You can add my Acked-by: Anton Altaparmakoc <an...@tuxera.com> line 
to it when sending it.  I am just saying that I think patches like that don't 
make much sense to me...

Best regards,

        Anton

> Regards
> Andrzej
> 
>> 
>> Best regards,
>> 
>>      Anton
>> 
>>> On 7 Aug 2015, at 08:59, Andrzej Hajda <a.ha...@samsung.com> wrote:
>>> 
>>> The patch was generated using fixed coccinelle semantic patch
>>> scripts/coccinelle/api/memdup.cocci [1].
>>> 
>>> [1]: http://permalink.gmane.org/gmane.linux.kernel/2014320
>>> 
>>> Signed-off-by: Andrzej Hajda <a.ha...@samsung.com>
>>> ---
>>> fs/ntfs/dir.c | 7 +++----
>>> 1 file changed, 3 insertions(+), 4 deletions(-)
>>> 
>>> diff --git a/fs/ntfs/dir.c b/fs/ntfs/dir.c
>>> index 9e38daf..2b7fef0 100644
>>> --- a/fs/ntfs/dir.c
>>> +++ b/fs/ntfs/dir.c
>>> @@ -1172,14 +1172,13 @@ static int ntfs_readdir(struct file *file, struct 
>>> dir_context *actor)
>>>      * map the mft record without deadlocking.
>>>      */
>>>     rc = le32_to_cpu(ctx->attr->data.resident.value_length);
>>> -   ir = kmalloc(rc, GFP_NOFS);
>>> +   /* Copy the index root value (it has been verified in read_inode). */
>>> +   ir = kmemdup((u8 *)ctx->attr + 
>>> le16_to_cpu(ctx->attr->data.resident.value_offset),
>>> +                rc, GFP_NOFS);
>>>     if (unlikely(!ir)) {
>>>             err = -ENOMEM;
>>>             goto err_out;
>>>     }
>>> -   /* Copy the index root value (it has been verified in read_inode). */
>>> -   memcpy(ir, (u8*)ctx->attr +
>>> -                   le16_to_cpu(ctx->attr->data.resident.value_offset), rc);
>>>     ntfs_attr_put_search_ctx(ctx);
>>>     unmap_mft_record(ndir);
>>>     ctx = NULL;
>>> -- 
>>> 1.9.1

-- 
Anton Altaparmakov <anton at tuxera.com> (replace at with @)
Lead in File System Development, Tuxera Inc., http://www.tuxera.com/
Linux NTFS maintainer

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to