From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753859AbcHRKEZ (ORCPT ); Thu, 18 Aug 2016 06:04:25 -0400 Received: from mail-wm0-f68.google.com ([74.125.82.68]:35146 "EHLO mail-wm0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753047AbcHRKEU (ORCPT ); Thu, 18 Aug 2016 06:04:20 -0400 Subject: Re: [PATCH 1/4] KVM-S390: Improve determination of sizes in kvm_s390_import_bp_data() To: Julia Lawall , walter harms References: <82b84c9c-38a4-4d17-910f-312668dbae01@users.sourceforge.net> <033d8595-d051-1fa8-95b1-5d2056eb5667@users.sourceforge.net> <57B562F3.1080004@bfs.de> Cc: SF Markus Elfring , kvm@vger.kernel.org, linux-s390@vger.kernel.org, =?UTF-8?Q?Christian_Borntr=c3=a4ger?= , Cornelia Huck , David Hildenbrand , Heiko Carstens , Martin Schwidefsky , =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= , LKML , kernel-janitors@vger.kernel.org From: Paolo Bonzini Message-ID: <9db1986a-9b93-72ca-f35e-85b5b5e9f351@redhat.com> Date: Thu, 18 Aug 2016 11:48:17 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 18/08/2016 11:02, Julia Lawall wrote: > > > On Thu, 18 Aug 2016, walter harms wrote: > >> >> >> Am 17.08.2016 20:06, schrieb SF Markus Elfring: >>> From: Markus Elfring >>> Date: Wed, 17 Aug 2016 18:29:04 +0200 >>> >>> Replace the specification of data structures by pointer dereferences >>> to make the corresponding size determination a bit safer according to >>> the Linux coding style convention. >>> >>> Signed-off-by: Markus Elfring >>> --- >>> arch/s390/kvm/guestdbg.c | 6 +++--- >>> 1 file changed, 3 insertions(+), 3 deletions(-) >>> >>> diff --git a/arch/s390/kvm/guestdbg.c b/arch/s390/kvm/guestdbg.c >>> index d1f8241..b68db4b 100644 >>> --- a/arch/s390/kvm/guestdbg.c >>> +++ b/arch/s390/kvm/guestdbg.c >>> @@ -216,7 +216,7 @@ int kvm_s390_import_bp_data(struct kvm_vcpu *vcpu, >>> else if (dbg->arch.nr_hw_bp > MAX_BP_COUNT) >>> return -EINVAL; >>> >>> - size = dbg->arch.nr_hw_bp * sizeof(struct kvm_hw_breakpoint); >>> + size = dbg->arch.nr_hw_bp * sizeof(*bp_data); >>> bp_data = kmalloc(size, GFP_KERNEL); >>> if (!bp_data) { >>> ret = -ENOMEM; >>> @@ -241,7 +241,7 @@ int kvm_s390_import_bp_data(struct kvm_vcpu *vcpu, >>> } >>> } >>> >>> - size = nr_wp * sizeof(struct kvm_hw_wp_info_arch); >>> + size = nr_wp * sizeof(*wp_info); >>> if (size > 0) { >>> wp_info = kmalloc(size, GFP_KERNEL); >>> if (!wp_info) { >>> @@ -249,7 +249,7 @@ int kvm_s390_import_bp_data(struct kvm_vcpu *vcpu, >>> goto error; >>> } >>> } >>> - size = nr_bp * sizeof(struct kvm_hw_bp_info_arch); >>> + size = nr_bp * sizeof(*bp_info); >>> if (size > 0) { >>> bp_info = kmalloc(size, GFP_KERNEL); >>> if (!bp_info) { >> >> >> IMHO the common pattern for kmalloc is >> bp_info = kmalloc( nr_bp * sizeof(*bp_info), GFP_KERNEL); >> >> i can not remember code with a check for size < 0, i guess it is here >> to avoid an overflow ? since kmalloc takes a size_t argument this would cause >> a malloc failure an can be ignored. > > Shoudn't it be kcalloc? Or kmalloc_array, since zeroing is not necessary. Might be an idea for a new Coccinelle script, like - kmalloc (N * sizeof T, GFP) + kmalloc_array(N, sizeof T, GFP) Thanks, Paolo > > julia > >> >> >> just my 2 cents. >> re, >> wh >> >