From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752763Ab2LDVXJ (ORCPT ); Tue, 4 Dec 2012 16:23:09 -0500 Received: from bsmtp5.bon.at ([195.3.86.187]:19693 "EHLO lbmfmo03.bon.at" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751722Ab2LDVXG (ORCPT ); Tue, 4 Dec 2012 16:23:06 -0500 From: Markus Grabner To: line6linux-devel@lists.sourceforge.net Cc: Stefan Hajnoczi , Laurent Navet , devel@driverdev.osuosl.org, Greg Kroah-Hartman , kernel-janitors@vger.kernel.org, linux-kernel Subject: Re: [Line6linux-devel] [PATCH] line6: Use kmemdup rather than duplicating its implementation Date: Tue, 04 Dec 2012 22:22:12 +0100 Message-ID: <4461097.8nbt8bgEO9@medialab> User-Agent: KMail/4.9.3 (Linux/3.4.11-2.16-desktop; KDE/4.9.3; x86_64; ; ) In-Reply-To: References: <1354540825-17735-1-git-send-email-laurent.navet@gmail.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Am Montag, 3. Dezember 2012, 17:34:07 schrieb Stefan Hajnoczi: > On Mon, Dec 3, 2012 at 2:20 PM, Laurent Navet wrote: > > staging: line6: driver.c > > > > The semantic patch that makes this output is available > > in scripts/coccinelle/api/memdup.cocci. > > > > Signed-off-by: Laurent Navet > > --- > > > > drivers/staging/line6/driver.c | 5 ++--- > > 1 file changed, 2 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/staging/line6/driver.c > > b/drivers/staging/line6/driver.c index f5c19b2..e1d6241 100644 > > --- a/drivers/staging/line6/driver.c > > +++ b/drivers/staging/line6/driver.c > > @@ -331,14 +331,13 @@ int line6_version_request_async(struct usb_line6 > > *line6)> > > char *buffer; > > int retval; > > > > - buffer = kmalloc(sizeof(line6_request_version), GFP_ATOMIC); > > + buffer = kmemdup(line6_request_version, > > + sizeof(line6_request_version), GFP_ATOMIC); > > > > if (buffer == NULL) { > > > > dev_err(line6->ifcdev, "Out of memory"); > > return -ENOMEM; > > > > } > > > > - memcpy(buffer, line6_request_version, > > sizeof(line6_request_version)); - > > > > retval = line6_send_raw_message_async(line6, buffer, > > > > sizeof(line6_request_version > > )); > > > > kfree(buffer); > > > > -- > > 1.7.10.4 > > Your change is fine but I'm not sure whether we should allocate memory > in the first place: I can't remember the precise reason for this copy operation, it was related to which type of memory is allowed for a URB data block, and memory declared with "static const char[]" at global scope in the driver is not allowed. I just verified on my system (kernel 3.4.11) that requesting the device's firmware version doesn't work when passing the line6_request_version pointer directly (instead of its kmemdup copy), so I think the kmemdup is necessary here. It's a bit unsatisfactory to make a copy just because the original data is not accessible for whatever reason, but I don't know of a better solution. Maybe somebody else can clarify this or propose an alternative method? Kind regards, Markus