| Message ID | 20150611154631.GA13202@mwanda (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
2015-06-12 0:46 GMT+09:00 Dan Carpenter <dan.carpenter@oracle.com>: > We allocate sizeof("-grp") which is 5 bytes but then we pass 4 to the > snprintf() so the last 'p' char is truncated away. > > The kzalloc() can be made into kmalloc() since we are going to fill > the whole buffer. But I know that Walter Harms is going to grumble if I > don't use kasprintf(). :P And also checkpatch.pl these days complains > that the "allocation failed" printks aren't needed so I removed them. > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > > diff --git a/drivers/pinctrl/samsung/pinctrl-exynos5440.c b/drivers/pinctrl/samsung/pinctrl-exynos5440.c > index f5619fb..f804a61c 100644 > --- a/drivers/pinctrl/samsung/pinctrl-exynos5440.c > +++ b/drivers/pinctrl/samsung/pinctrl-exynos5440.c > @@ -215,12 +215,9 @@ static int exynos5440_dt_node_to_map(struct pinctrl_dev *pctldev, > * Allocate memory for pin group name. The pin group name is derived > * from the node name from which these map entries are be created. > */ > - gname = kzalloc(strlen(np->name) + GSUFFIX_LEN, GFP_KERNEL); > - if (!gname) { > - dev_err(dev, "failed to alloc memory for group name\n"); > + gname = kasprintf(GFP_KERNEL, "%s%s", np->name, GROUP_SUFFIX); > + if (!gname) > goto free_map; > - } > - snprintf(gname, strlen(np->name) + 4, "%s%s", np->name, GROUP_SUFFIX); I would prefer splitting this into two patches: one for fixing truncated name (by usage of kasprintf) and second for ENOMEM message. The second patch can get rid of ENOMEM message in other places as well. As for the first issue I think that function suffix would also be truncated in the same way. Best regards, Krzysztof -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jun 15, 2015 at 01:17:36PM +0900, Krzysztof Kozlowski wrote: > I would prefer splitting this into two patches: one for fixing > truncated name (by usage of kasprintf) and second for ENOMEM message. > The second patch can get rid of ENOMEM message in other places as > well. > > As for the first issue I think that function suffix would also be > truncated in the same way. Sorry for the delayed response, I've been offline for a few days. Yeah, you're right. Let me resend. regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/pinctrl/samsung/pinctrl-exynos5440.c b/drivers/pinctrl/samsung/pinctrl-exynos5440.c index f5619fb..f804a61c 100644 --- a/drivers/pinctrl/samsung/pinctrl-exynos5440.c +++ b/drivers/pinctrl/samsung/pinctrl-exynos5440.c @@ -215,12 +215,9 @@ static int exynos5440_dt_node_to_map(struct pinctrl_dev *pctldev, * Allocate memory for pin group name. The pin group name is derived * from the node name from which these map entries are be created. */ - gname = kzalloc(strlen(np->name) + GSUFFIX_LEN, GFP_KERNEL); - if (!gname) { - dev_err(dev, "failed to alloc memory for group name\n"); + gname = kasprintf(GFP_KERNEL, "%s%s", np->name, GROUP_SUFFIX); + if (!gname) goto free_map; - } - snprintf(gname, strlen(np->name) + 4, "%s%s", np->name, GROUP_SUFFIX); /* * don't have config options? then skip over to creating function @@ -710,14 +707,10 @@ static int exynos5440_pinctrl_parse_dt(struct platform_device *pdev, } /* derive pin group name from the node name */ - gname = devm_kzalloc(dev, strlen(cfg_np->name) + GSUFFIX_LEN, - GFP_KERNEL); - if (!gname) { - dev_err(dev, "failed to alloc memory for group name\n"); + gname = devm_kasprintf(dev, GFP_KERNEL, + "%s%s", cfg_np->name, GROUP_SUFFIX); + if (!gname) return -ENOMEM; - } - snprintf(gname, strlen(cfg_np->name) + 4, "%s%s", cfg_np->name, - GROUP_SUFFIX); grp->name = gname; grp->pins = pin_list;
We allocate sizeof("-grp") which is 5 bytes but then we pass 4 to the snprintf() so the last 'p' char is truncated away. The kzalloc() can be made into kmalloc() since we are going to fill the whole buffer. But I know that Walter Harms is going to grumble if I don't use kasprintf(). :P And also checkpatch.pl these days complains that the "allocation failed" printks aren't needed so I removed them. Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html