* [patch] drm/amd: cleanup get_mfd_cell_dev() @ 2016-02-25 7:47 Dan Carpenter 2016-02-26 17:46 ` Alex Deucher 2016-02-27 9:50 ` walter harms 0 siblings, 2 replies; 5+ messages in thread From: Dan Carpenter @ 2016-02-25 7:47 UTC (permalink / raw) To: David Airlie Cc: Maruthi Srinivas Bayyavarapu, Jammy Zhou, kernel-janitors, linux-kernel, dri-devel, Alex Deucher, Murali Krishna Vemuri 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 <dan.carpenter@oracle.com> 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); _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [patch] drm/amd: cleanup get_mfd_cell_dev() 2016-02-25 7:47 [patch] drm/amd: cleanup get_mfd_cell_dev() Dan Carpenter @ 2016-02-26 17:46 ` Alex Deucher 2016-02-27 9:50 ` walter harms 1 sibling, 0 replies; 5+ messages in thread From: Alex Deucher @ 2016-02-26 17:46 UTC (permalink / raw) To: Dan Carpenter Cc: Maruthi Srinivas Bayyavarapu, Jammy Zhou, kernel-janitors, LKML, Maling list - DRI developers, Alex Deucher, Murali Krishna Vemuri On Thu, Feb 25, 2016 at 2:47 AM, Dan Carpenter <dan.carpenter@oracle.com> wrote: > 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 <dan.carpenter@oracle.com> Applied. Thanks! > > 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); > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [patch] drm/amd: cleanup get_mfd_cell_dev() 2016-02-25 7:47 [patch] drm/amd: cleanup get_mfd_cell_dev() Dan Carpenter 2016-02-26 17:46 ` Alex Deucher @ 2016-02-27 9:50 ` walter harms 2016-02-27 10:40 ` Dan Carpenter 1 sibling, 1 reply; 5+ messages in thread From: walter harms @ 2016-02-27 9:50 UTC (permalink / raw) To: Dan Carpenter Cc: David Airlie, Alex Deucher, Maruthi Srinivas Bayyavarapu, Jammy Zhou, Murali Krishna Vemuri, Chunming Zhou, dri-devel, linux-kernel, kernel-janitors 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 <dan.carpenter@oracle.com> > > 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. re, wh ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [patch] drm/amd: cleanup get_mfd_cell_dev() 2016-02-27 9:50 ` walter harms @ 2016-02-27 10:40 ` Dan Carpenter 2016-02-27 11:18 ` walter harms 0 siblings, 1 reply; 5+ messages in thread From: Dan Carpenter @ 2016-02-27 10:40 UTC (permalink / raw) To: walter harms Cc: Maruthi Srinivas Bayyavarapu, Jammy Zhou, kernel-janitors, linux-kernel, dri-devel, 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 <dan.carpenter@oracle.com> > > > > 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 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [patch] drm/amd: cleanup get_mfd_cell_dev() 2016-02-27 10:40 ` Dan Carpenter @ 2016-02-27 11:18 ` walter harms 0 siblings, 0 replies; 5+ messages in thread From: walter harms @ 2016-02-27 11:18 UTC (permalink / raw) To: Dan Carpenter Cc: Maruthi Srinivas Bayyavarapu, Jammy Zhou, kernel-janitors, linux-kernel, dri-devel, Alex Deucher, Murali Krishna Vemuri Am 27.02.2016 11:40, schrieb Dan Carpenter: > 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 <dan.carpenter@oracle.com> >>> >>> 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. > Hi Dan, i also think that this limit is artificial, one of those "it works" things. The problem is that i have seen changes in the naming als ready done, like the shift from /dev/sda to things like /dev/disk/by-id/scsi-SATA_WDC_WD5000AAKS-_WD-WCASY5869245-part1 and you are out of the game here. snprintf will cut the tail and the %d.auto stuff is dead in the water. To make it clear, I do not thing that is security related issue but it could result in annoying failures. (the solution is obviously to use asprintf() and free the mem later :) ). re, wh _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-02-27 11:18 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-02-25 7:47 [patch] drm/amd: cleanup get_mfd_cell_dev() Dan Carpenter 2016-02-26 17:46 ` Alex Deucher 2016-02-27 9:50 ` walter harms 2016-02-27 10:40 ` Dan Carpenter 2016-02-27 11:18 ` walter harms
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).