From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-s390-owner@vger.kernel.org>
Received: from mout.web.de ([212.227.17.12]:60689 "EHLO mout.web.de"
        rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
        id S1730392AbfKFNAz (ORCPT <rfc822;linux-s390@vger.kernel.org>);
        Wed, 6 Nov 2019 08:00:55 -0500
Subject: Re: s390/pkey: Use memdup_user() rather than duplicating its
 implementation
References: <08422b7e-2071-ee52-049e-c3ac55bc67a9@web.de>
 <6137855bb4170c438c7436cbdb7dfd21639a8855.camel@perches.com>
From: Markus Elfring <Markus.Elfring@web.de>
Message-ID: <4fa0e106-2565-8610-1356-4adfba08c0a0@web.de>
Date: Wed, 6 Nov 2019 14:00:33 +0100
MIME-Version: 1.0
In-Reply-To: <6137855bb4170c438c7436cbdb7dfd21639a8855.camel@perches.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: quoted-printable
Sender: linux-s390-owner@vger.kernel.org
List-ID: <linux-s390.vger.kernel.org>
To: Joe Perches <joe@perches.com>, linux-s390@vger.kernel.org, kernel-janitors@vger.kernel.org
Cc: =?UTF-8?Q?Christian_Borntr=c3=a4ger?= <borntraeger@de.ibm.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>, LKML <linux-kernel@vger.kernel.org>

>> Reuse existing functionality from memdup_user() instead of keeping
>> duplicate source code.
>>
>> Generated by: scripts/coccinelle/api/memdup_user.cocci
=E2=80=A6
>> Fixes: f2bbc96e7cfad3891b7bf9bd3e566b9b7ab4553d ("s390/pkey: add CCA AE=
S cipher key support")
>
> This doesn't fix anything

How would you categorise the proposed source code reduction and software r=
euse?


> and the Fixes: line is not appropriate.

Will the development opinions vary between contributors?


>> +	return !ukey || keylen < MINKEYBLOBSIZE || keylen > KEYBLOBBUFSIZE
>> +	       ? ERR_PTR(-EINVAL)
>> +	       : memdup_user(ukey, keylen);
>
> This is a very poor use of ternary ?: code.

The conditional operator is applied once more in the intended way,
isn't it?


> This is much more readable for humans.

Readability preferences can vary also for this code structure.


>> +	return uapqns && nr_apqns > 0
>> +	       ? memdup_user(uapqns, nr_apqns * sizeof(struct pkey_apqn))
>> +	       : NULL;
>
> And here you reverse the form of the earlier block.

I kept the previous condition specification.


> Please be consistent and use this style:

Would further developers like to get a more verbose variant for this
software transformation?

Regards,
Markus