All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christian Borntraeger <borntraeger@de.ibm.com>
To: Markus Elfring <Markus.Elfring@web.de>,
	linux-s390@vger.kernel.org, Joe Perches <joe@perches.com>,
	Harald Freudenberger <freude@linux.ibm.com>,
	Heiko Carstens <heiko.carstens@de.ibm.com>,
	Ingo Franzki <ifranzki@linux.ibm.com>,
	Vasily Gorbik <gor@linux.ibm.com>
Cc: LKML <linux-kernel@vger.kernel.org>,
	kernel-janitors@vger.kernel.org, Kangjie Lu <kjlu@umn.edu>,
	Navid Emamdoost <emamd001@umn.edu>,
	Stephen McCamant <smccaman@umn.edu>
Subject: Re: s390/pkey: Use memdup_user() rather than duplicating its implementation
Date: Fri, 8 Nov 2019 12:32:42 +0100	[thread overview]
Message-ID: <c0df9cc8-c41a-1e5d-811c-1ff045c13fcc@de.ibm.com> (raw)
In-Reply-To: <8f98f9fc-57df-5993-44b5-5ea4c0de7ef9@web.de>



On 07.11.19 15:27, Markus Elfring wrote:
>>>>> Reuse existing functionality from memdup_user() instead of keeping
>>>>> duplicate source code.
>>>>>
>>>>> Generated by: scripts/coccinelle/api/memdup_user.cocci
>>>>>
>>>>> Delete local variables which became unnecessary with this refactoring
>>>>> in two function implementations.
>>>>>
>>>>> Fixes: f2bbc96e7cfad3891b7bf9bd3e566b9b7ab4553d ("s390/pkey: add CCA AES cipher key support")
>>>>
>>>> With that patch description, the Fixes tag is wrong...but (see below)
>>>
>>> I wonder about such a conclusion together with your subsequent feedback.
>>
>> Please try to read and understand what other people write.
> 
> I am also trying as usual.
> 
> 
>> My point was that your patch description only talks about refactoring
>> and avoiding code duplication.
> 
> These implementation details are mentioned.

Exactly and my point is that the main value of your patch is not the refactoring,
but the fact that your refactoring uncovered an existing memory leak. The refactoring
itself is usually not a fix.


So can you just redo the patch with a new patch description ala,

refactoring and reuse. While doing this this also uncovered a real code
bug (memory leak) that is fixed by the refactoring.

And please do that without continue this discussion,


WARNING: multiple messages have this Message-ID (diff)
From: Christian Borntraeger <borntraeger@de.ibm.com>
To: Markus Elfring <Markus.Elfring@web.de>,
	linux-s390@vger.kernel.org, Joe Perches <joe@perches.com>,
	Harald Freudenberger <freude@linux.ibm.com>,
	Heiko Carstens <heiko.carstens@de.ibm.com>,
	Ingo Franzki <ifranzki@linux.ibm.com>,
	Vasily Gorbik <gor@linux.ibm.com>
Cc: LKML <linux-kernel@vger.kernel.org>,
	kernel-janitors@vger.kernel.org, Kangjie Lu <kjlu@umn.edu>,
	Navid Emamdoost <emamd001@umn.edu>,
	Stephen McCamant <smccaman@umn.edu>
Subject: Re: s390/pkey: Use memdup_user() rather than duplicating its implementation
Date: Fri, 08 Nov 2019 11:32:42 +0000	[thread overview]
Message-ID: <c0df9cc8-c41a-1e5d-811c-1ff045c13fcc@de.ibm.com> (raw)
In-Reply-To: <8f98f9fc-57df-5993-44b5-5ea4c0de7ef9@web.de>



On 07.11.19 15:27, Markus Elfring wrote:
>>>>> Reuse existing functionality from memdup_user() instead of keeping
>>>>> duplicate source code.
>>>>>
>>>>> Generated by: scripts/coccinelle/api/memdup_user.cocci
>>>>>
>>>>> Delete local variables which became unnecessary with this refactoring
>>>>> in two function implementations.
>>>>>
>>>>> Fixes: f2bbc96e7cfad3891b7bf9bd3e566b9b7ab4553d ("s390/pkey: add CCA AES cipher key support")
>>>>
>>>> With that patch description, the Fixes tag is wrong...but (see below)
>>>
>>> I wonder about such a conclusion together with your subsequent feedback.
>>
>> Please try to read and understand what other people write.
> 
> I am also trying as usual.
> 
> 
>> My point was that your patch description only talks about refactoring
>> and avoiding code duplication.
> 
> These implementation details are mentioned.

Exactly and my point is that the main value of your patch is not the refactoring,
but the fact that your refactoring uncovered an existing memory leak. The refactoring
itself is usually not a fix.


So can you just redo the patch with a new patch description ala,

refactoring and reuse. While doing this this also uncovered a real code
bug (memory leak) that is fixed by the refactoring.

And please do that without continue this discussion,

  reply	other threads:[~2019-11-08 11:32 UTC|newest]

Thread overview: 62+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-06 10:22 [PATCH] s390/pkey: Use memdup_user() rather than duplicating its implementation Markus Elfring
2019-11-06 10:22 ` Markus Elfring
2019-11-06 10:38 ` Joe Perches
2019-11-06 10:38   ` Joe Perches
2019-11-06 13:00   ` Markus Elfring
2019-11-06 13:00     ` Markus Elfring
2019-11-06 17:29     ` Joe Perches
2019-11-06 17:29       ` Joe Perches
2019-11-06 18:55       ` Markus Elfring
2019-11-06 18:55         ` Markus Elfring
2019-11-06 19:01         ` Joe Perches
2019-11-06 19:01           ` Joe Perches
2019-11-06 19:18           ` Markus Elfring
2019-11-06 19:18             ` Markus Elfring
2019-11-06 13:00   ` Markus Elfring
2019-11-06 13:00     ` Markus Elfring
2019-11-06 18:30     ` Christian Borntraeger
2019-11-06 18:30       ` Christian Borntraeger
2019-11-07  6:48       ` Dan Carpenter
2019-11-07  6:48         ` Dan Carpenter
2019-11-07  8:07         ` Christian Borntraeger
2019-11-07  8:07           ` Christian Borntraeger
2019-11-07 10:06   ` [PATCH v2] " Markus Elfring
2019-11-07 10:06     ` Markus Elfring
2019-11-07 12:44     ` Christian Borntraeger
2019-11-07 12:44       ` Christian Borntraeger
2019-11-07 13:45       ` Markus Elfring
2019-11-07 13:45         ` Markus Elfring
2019-11-07 13:54         ` Christian Borntraeger
2019-11-07 13:54           ` Christian Borntraeger
2019-11-07 14:27           ` Markus Elfring
2019-11-07 14:27             ` Markus Elfring
2019-11-08 11:32             ` Christian Borntraeger [this message]
2019-11-08 11:32               ` Christian Borntraeger
2019-11-08 17:14               ` [PATCH v3] " Markus Elfring
2019-11-08 17:14                 ` Markus Elfring
2019-11-11  7:54                 ` Christian Borntraeger
2019-11-11  7:54                   ` Christian Borntraeger
2019-11-11  8:11                   ` [v3] " Markus Elfring
2019-11-11  8:11                     ` Markus Elfring
2019-11-11  8:27                     ` Christian Borntraeger
2019-11-11  8:27                       ` Christian Borntraeger
2019-11-11  8:42                       ` Markus Elfring
2019-11-11  8:42                         ` Markus Elfring
2019-11-11  8:56                         ` Christian Borntraeger
2019-11-11  8:56                           ` Christian Borntraeger
2019-11-11  9:06                           ` Markus Elfring
2019-11-11  9:06                             ` Markus Elfring
2019-11-11  9:08                             ` Christian Borntraeger
2019-11-11  9:08                               ` Christian Borntraeger
2019-11-11  9:17                               ` Markus Elfring
2019-11-11  9:17                                 ` Markus Elfring
2019-11-11  9:18                                 ` Christian Borntraeger
2019-11-11  9:18                                   ` Christian Borntraeger
2019-11-11  9:26                                   ` Markus Elfring
2019-11-11  9:26                                     ` Markus Elfring
2019-11-11 14:45                   ` [PATCH v4] s390/pkey: Fix memory leak in error case by using memdup_user() rather than open coding Markus Elfring
2019-11-11 14:45                     ` Markus Elfring
2019-11-11 16:40                     ` Christian Borntraeger
2019-11-11 16:40                       ` [PATCH v4] s390/pkey: Fix memory leak in error case by using memdup_user() rather than open codi Christian Borntraeger
2019-11-13 17:09                       ` [v4] s390/pkey: Fix memory leak in error case by using memdup_user() rather than open coding Markus Elfring
2019-11-13 17:09                         ` Markus Elfring

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=c0df9cc8-c41a-1e5d-811c-1ff045c13fcc@de.ibm.com \
    --to=borntraeger@de.ibm.com \
    --cc=Markus.Elfring@web.de \
    --cc=emamd001@umn.edu \
    --cc=freude@linux.ibm.com \
    --cc=gor@linux.ibm.com \
    --cc=heiko.carstens@de.ibm.com \
    --cc=ifranzki@linux.ibm.com \
    --cc=joe@perches.com \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=kjlu@umn.edu \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-s390@vger.kernel.org \
    --cc=smccaman@umn.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.