From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dan Carpenter Date: Sat, 27 Feb 2016 10:40:49 +0000 Subject: Re: [patch] drm/amd: cleanup get_mfd_cell_dev() Message-Id: <20160227104049.GN5273@mwanda> List-Id: References: <20160225074709.GA7333@mwanda> <56D17170.80408@bfs.de> In-Reply-To: <56D17170.80408@bfs.de> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: walter harms Cc: Maruthi Srinivas Bayyavarapu , Jammy Zhou , kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, Alex Deucher , Murali Krishna Vemuri On Sat, Feb 27, 2016 at 10:50:40AM +0100, walter harms wrote: > > > Am 25.02.2016 08:47, schrieb Dan Carpenter: > > It's simpler to just use snprintf() to print this to one buffer instead > > of using strcpy() and strcat(). Also using snprintf() is slightly safer > > than using sprintf(). > > > > Signed-off-by: Dan Carpenter > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c > > index 9f8cfaa..d6b0bff 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c > > @@ -240,12 +240,10 @@ static int acp_poweron(struct generic_pm_domain *genpd) > > static struct device *get_mfd_cell_dev(const char *device_name, int r) > > { > > char auto_dev_name[25]; > > - char buf[8]; > > struct device *dev; > > > > - sprintf(buf, ".%d.auto", r); > > - strcpy(auto_dev_name, device_name); > > - strcat(auto_dev_name, buf); > > + snprintf(auto_dev_name, sizeof(auto_dev_name), > > + "%s.%d.auto", device_name, r); > > dev = bus_find_device_by_name(&platform_bus_type, NULL, auto_dev_name); > > dev_info(dev, "device %s added to pm domain\n", auto_dev_name); > > > > hi, > i tried to understand what is the base for char auto_dev_name[25]. It is not clear > from these snipped if that is large or small. > > (To be aware i assume that > get_mfd_cell_dev("terrible_long_and_Stupid_name",1234567899346712) will never happen > but i could find no reason) > > A small comment that explains the magic 25 would be nice. I have no idea, either of course. For example, mc13xxx_add_subdevice_pdata() assumes device_name by itself can be 30 characters. Hence the change to snprintf. regards, dan carpenter From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dan Carpenter Subject: Re: [patch] drm/amd: cleanup get_mfd_cell_dev() Date: Sat, 27 Feb 2016 13:40:49 +0300 Message-ID: <20160227104049.GN5273@mwanda> References: <20160225074709.GA7333@mwanda> <56D17170.80408@bfs.de> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from aserp1040.oracle.com (aserp1040.oracle.com [141.146.126.69]) by gabe.freedesktop.org (Postfix) with ESMTPS id D142B6E0B7 for ; Sat, 27 Feb 2016 10:41:15 +0000 (UTC) Content-Disposition: inline In-Reply-To: <56D17170.80408@bfs.de> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: walter harms Cc: Maruthi Srinivas Bayyavarapu , Jammy Zhou , kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, Alex Deucher , Murali Krishna Vemuri List-Id: dri-devel@lists.freedesktop.org T24gU2F0LCBGZWIgMjcsIDIwMTYgYXQgMTA6NTA6NDBBTSArMDEwMCwgd2FsdGVyIGhhcm1zIHdy b3RlOgo+IAo+IAo+IEFtIDI1LjAyLjIwMTYgMDg6NDcsIHNjaHJpZWIgRGFuIENhcnBlbnRlcjoK PiA+IEl0J3Mgc2ltcGxlciB0byBqdXN0IHVzZSBzbnByaW50ZigpIHRvIHByaW50IHRoaXMgdG8g b25lIGJ1ZmZlciBpbnN0ZWFkCj4gPiBvZiB1c2luZyBzdHJjcHkoKSBhbmQgc3RyY2F0KCkuICBB bHNvIHVzaW5nIHNucHJpbnRmKCkgaXMgc2xpZ2h0bHkgc2FmZXIKPiA+IHRoYW4gdXNpbmcgc3By aW50ZigpLgo+ID4gCj4gPiBTaWduZWQtb2ZmLWJ5OiBEYW4gQ2FycGVudGVyIDxkYW4uY2FycGVu dGVyQG9yYWNsZS5jb20+Cj4gPiAKPiA+IGRpZmYgLS1naXQgYS9kcml2ZXJzL2dwdS9kcm0vYW1k L2FtZGdwdS9hbWRncHVfYWNwLmMgYi9kcml2ZXJzL2dwdS9kcm0vYW1kL2FtZGdwdS9hbWRncHVf YWNwLmMKPiA+IGluZGV4IDlmOGNmYWEuLmQ2YjBiZmYgMTAwNjQ0Cj4gPiAtLS0gYS9kcml2ZXJz L2dwdS9kcm0vYW1kL2FtZGdwdS9hbWRncHVfYWNwLmMKPiA+ICsrKyBiL2RyaXZlcnMvZ3B1L2Ry bS9hbWQvYW1kZ3B1L2FtZGdwdV9hY3AuYwo+ID4gQEAgLTI0MCwxMiArMjQwLDEwIEBAIHN0YXRp YyBpbnQgYWNwX3Bvd2Vyb24oc3RydWN0IGdlbmVyaWNfcG1fZG9tYWluICpnZW5wZCkKPiA+ICBz dGF0aWMgc3RydWN0IGRldmljZSAqZ2V0X21mZF9jZWxsX2Rldihjb25zdCBjaGFyICpkZXZpY2Vf bmFtZSwgaW50IHIpCj4gPiAgewo+ID4gIAljaGFyIGF1dG9fZGV2X25hbWVbMjVdOwo+ID4gLQlj aGFyIGJ1Zls4XTsKPiA+ICAJc3RydWN0IGRldmljZSAqZGV2Owo+ID4gIAo+ID4gLQlzcHJpbnRm KGJ1ZiwgIi4lZC5hdXRvIiwgcik7Cj4gPiAtCXN0cmNweShhdXRvX2Rldl9uYW1lLCBkZXZpY2Vf bmFtZSk7Cj4gPiAtCXN0cmNhdChhdXRvX2Rldl9uYW1lLCBidWYpOwo+ID4gKwlzbnByaW50Zihh dXRvX2Rldl9uYW1lLCBzaXplb2YoYXV0b19kZXZfbmFtZSksCj4gPiArCQkgIiVzLiVkLmF1dG8i LCBkZXZpY2VfbmFtZSwgcik7Cj4gPiAgCWRldiA9IGJ1c19maW5kX2RldmljZV9ieV9uYW1lKCZw bGF0Zm9ybV9idXNfdHlwZSwgTlVMTCwgYXV0b19kZXZfbmFtZSk7Cj4gPiAgCWRldl9pbmZvKGRl diwgImRldmljZSAlcyBhZGRlZCB0byBwbSBkb21haW5cbiIsIGF1dG9fZGV2X25hbWUpOwo+ID4g IAo+IAo+IGhpLAo+IGkgdHJpZWQgdG8gdW5kZXJzdGFuZCB3aGF0IGlzIHRoZSBiYXNlIGZvciBj aGFyIGF1dG9fZGV2X25hbWVbMjVdLiBJdCBpcyBub3QgY2xlYXIKPiBmcm9tIHRoZXNlIHNuaXBw ZWQgaWYgdGhhdCBpcyBsYXJnZSBvciBzbWFsbC4KPiAKPiAoVG8gYmUgYXdhcmUgaSBhc3N1bWUg dGhhdAo+IGdldF9tZmRfY2VsbF9kZXYoInRlcnJpYmxlX2xvbmdfYW5kX1N0dXBpZF9uYW1lIiwx MjM0NTY3ODk5MzQ2NzEyKSB3aWxsIG5ldmVyIGhhcHBlbgo+IGJ1dCBpIGNvdWxkIGZpbmQgbm8g cmVhc29uKQo+IAo+IEEgc21hbGwgY29tbWVudCB0aGF0IGV4cGxhaW5zIHRoZSBtYWdpYyAyNSB3 b3VsZCBiZSBuaWNlLgoKSSBoYXZlIG5vIGlkZWEsIGVpdGhlciBvZiBjb3Vyc2UuICBGb3IgZXhh bXBsZSwKbWMxM3h4eF9hZGRfc3ViZGV2aWNlX3BkYXRhKCkgYXNzdW1lcyBkZXZpY2VfbmFtZSBi eSBpdHNlbGYgY2FuIGJlIDMwCmNoYXJhY3RlcnMuICBIZW5jZSB0aGUgY2hhbmdlIHRvIHNucHJp bnRmLgoKcmVnYXJkcywKZGFuIGNhcnBlbnRlcgoKX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVsIG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlz dHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlzdHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4v bGlzdGluZm8vZHJpLWRldmVsCg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1945938AbcB0KlS (ORCPT ); Sat, 27 Feb 2016 05:41:18 -0500 Received: from aserp1040.oracle.com ([141.146.126.69]:49253 "EHLO aserp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756115AbcB0KlQ (ORCPT ); Sat, 27 Feb 2016 05:41:16 -0500 Date: Sat, 27 Feb 2016 13:40:49 +0300 From: Dan Carpenter To: walter harms Cc: David Airlie , Alex Deucher , Maruthi Srinivas Bayyavarapu , Jammy Zhou , Murali Krishna Vemuri , Chunming Zhou , dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org Subject: Re: [patch] drm/amd: cleanup get_mfd_cell_dev() Message-ID: <20160227104049.GN5273@mwanda> References: <20160225074709.GA7333@mwanda> <56D17170.80408@bfs.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <56D17170.80408@bfs.de> User-Agent: Mutt/1.5.21 (2010-09-15) X-Source-IP: aserv0021.oracle.com [141.146.126.233] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sat, Feb 27, 2016 at 10:50:40AM +0100, walter harms wrote: > > > Am 25.02.2016 08:47, schrieb Dan Carpenter: > > It's simpler to just use snprintf() to print this to one buffer instead > > of using strcpy() and strcat(). Also using snprintf() is slightly safer > > than using sprintf(). > > > > Signed-off-by: Dan Carpenter > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c > > index 9f8cfaa..d6b0bff 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c > > @@ -240,12 +240,10 @@ static int acp_poweron(struct generic_pm_domain *genpd) > > static struct device *get_mfd_cell_dev(const char *device_name, int r) > > { > > char auto_dev_name[25]; > > - char buf[8]; > > struct device *dev; > > > > - sprintf(buf, ".%d.auto", r); > > - strcpy(auto_dev_name, device_name); > > - strcat(auto_dev_name, buf); > > + snprintf(auto_dev_name, sizeof(auto_dev_name), > > + "%s.%d.auto", device_name, r); > > dev = bus_find_device_by_name(&platform_bus_type, NULL, auto_dev_name); > > dev_info(dev, "device %s added to pm domain\n", auto_dev_name); > > > > hi, > i tried to understand what is the base for char auto_dev_name[25]. It is not clear > from these snipped if that is large or small. > > (To be aware i assume that > get_mfd_cell_dev("terrible_long_and_Stupid_name",1234567899346712) will never happen > but i could find no reason) > > A small comment that explains the magic 25 would be nice. I have no idea, either of course. For example, mc13xxx_add_subdevice_pdata() assumes device_name by itself can be 30 characters. Hence the change to snprintf. regards, dan carpenter