Patchwork drivers/of: Constify device_node->name and ->path_component_name

login
register
mail settings
Submitter Grant Likely
Date Nov. 14, 2012, 10:28 p.m.
Message ID <1352932107-14782-1-git-send-email-grant.likely@secretlab.ca>
Download mbox | patch
Permalink /patch/199039/
State Not Applicable
Delegated to: David Miller
Headers show

Comments

Grant Likely - Nov. 14, 2012, 10:28 p.m.
Neither of these should ever be changed once set. Make them const

Build tested with defconfigs on ARM, PowerPC, Sparc, MIPS, x86 among
others.

Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: "David S. Miller" <davem@davemloft.net>
---
 arch/powerpc/platforms/powermac/pfunc_core.c |    2 +-
 arch/powerpc/platforms/pseries/reconfig.c    |    3 +--
 arch/powerpc/sysdev/fsl_pci.c                |    2 +-
 arch/sparc/kernel/pci_impl.h                 |    2 +-
 drivers/of/fdt.c                             |   10 +++++-----
 include/linux/of.h                           |    4 ++--
 6 files changed, 11 insertions(+), 12 deletions(-)
David Miller - Nov. 14, 2012, 10:33 p.m.
You're making other changes here, such as the kstrdup() stuff,
seperate that into another patch.
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Grant Likely - Nov. 14, 2012, 10:42 p.m.
On Wed, Nov 14, 2012 at 10:33 PM, David Miller <davem@davemloft.net> wrote:
>
> You're making other changes here, such as the kstrdup() stuff,
> seperate that into another patch.

It's part of the same change. The original code was allocating a
buffer, saving the pointer in the name field and then modifying it.
Making the code to a kstrdup() gets rid of modifying the const buffer.

g.
Julian Calaby - Nov. 14, 2012, 10:46 p.m.
Hi Grant,

On Thu, Nov 15, 2012 at 9:42 AM, Grant Likely <grant.likely@secretlab.ca> wrote:
> On Wed, Nov 14, 2012 at 10:33 PM, David Miller <davem@davemloft.net> wrote:
>>
>> You're making other changes here, such as the kstrdup() stuff,
>> seperate that into another patch.
>
> It's part of the same change. The original code was allocating a
> buffer, saving the pointer in the name field and then modifying it.
> Making the code to a kstrdup() gets rid of modifying the const buffer.

You should really mention this in the commit log, maybe something like:


[PATCH] drivers/of: Constify device_node->name and ->path_component_name

Neither of these should ever be changed once set. Make them const

Also use kstrdup() instead of memcpy() so we don't attempt to modify them.

Build tested with defconfigs on ARM, PowerPC, Sparc, MIPS, x86 among
others.


Thanks,
Grant Likely - Nov. 14, 2012, 10:48 p.m.
On Wed, Nov 14, 2012 at 10:46 PM, Julian Calaby <julian.calaby@gmail.com> wrote:
> Hi Grant,
>
> On Thu, Nov 15, 2012 at 9:42 AM, Grant Likely <grant.likely@secretlab.ca> wrote:
>> On Wed, Nov 14, 2012 at 10:33 PM, David Miller <davem@davemloft.net> wrote:
>>>
>>> You're making other changes here, such as the kstrdup() stuff,
>>> seperate that into another patch.
>>
>> It's part of the same change. The original code was allocating a
>> buffer, saving the pointer in the name field and then modifying it.
>> Making the code to a kstrdup() gets rid of modifying the const buffer.
>
> You should really mention this in the commit log, maybe something like:
>
>
> [PATCH] drivers/of: Constify device_node->name and ->path_component_name
>
> Neither of these should ever be changed once set. Make them const
>
> Also use kstrdup() instead of memcpy() so we don't attempt to modify them.

Yup, will do.

g.
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller - Nov. 14, 2012, 10:49 p.m.
From: Grant Likely <grant.likely@secretlab.ca>
Date: Wed, 14 Nov 2012 22:42:26 +0000

> On Wed, Nov 14, 2012 at 10:33 PM, David Miller <davem@davemloft.net> wrote:
>>
>> You're making other changes here, such as the kstrdup() stuff,
>> seperate that into another patch.
> 
> It's part of the same change. The original code was allocating a
> buffer, saving the pointer in the name field and then modifying it.
> Making the code to a kstrdup() gets rid of modifying the const buffer.

Please mention that in your commit message then, thanks.
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/arch/powerpc/platforms/powermac/pfunc_core.c b/arch/powerpc/platforms/powermac/pfunc_core.c
index b0c3777..d588e48 100644
--- a/arch/powerpc/platforms/powermac/pfunc_core.c
+++ b/arch/powerpc/platforms/powermac/pfunc_core.c
@@ -686,7 +686,7 @@  static int pmf_add_functions(struct pmf_device *dev, void *driverdata)
 	int count = 0;
 
 	for (pp = dev->node->properties; pp != 0; pp = pp->next) {
-		char *name;
+		const char *name;
 		if (strncmp(pp->name, PP_PREFIX, plen) != 0)
 			continue;
 		name = pp->name + plen;
diff --git a/arch/powerpc/platforms/pseries/reconfig.c b/arch/powerpc/platforms/pseries/reconfig.c
index 39f71fb..2f46681 100644
--- a/arch/powerpc/platforms/pseries/reconfig.c
+++ b/arch/powerpc/platforms/pseries/reconfig.c
@@ -281,12 +281,11 @@  static struct property *new_property(const char *name, const int length,
 	if (!new)
 		return NULL;
 
-	if (!(new->name = kmalloc(strlen(name) + 1, GFP_KERNEL)))
+	if (!(new->name = kstrdup(name, GFP_KERNEL)))
 		goto cleanup;
 	if (!(new->value = kmalloc(length + 1, GFP_KERNEL)))
 		goto cleanup;
 
-	strcpy(new->name, name);
 	memcpy(new->value, value, length);
 	*(((char *)new->value) + length) = 0;
 	new->length = length;
diff --git a/arch/powerpc/sysdev/fsl_pci.c b/arch/powerpc/sysdev/fsl_pci.c
index ffb93ae..01b62a6 100644
--- a/arch/powerpc/sysdev/fsl_pci.c
+++ b/arch/powerpc/sysdev/fsl_pci.c
@@ -136,7 +136,7 @@  static void __init setup_pci_atmu(struct pci_controller *hose,
 	u32 pcicsrbar = 0, pcicsrbar_sz;
 	u32 piwar = PIWAR_EN | PIWAR_PF | PIWAR_TGI_LOCAL |
 			PIWAR_READ_SNOOP | PIWAR_WRITE_SNOOP;
-	char *name = hose->dn->full_name;
+	const char *name = hose->dn->full_name;
 	const u64 *reg;
 	int len;
 
diff --git a/arch/sparc/kernel/pci_impl.h b/arch/sparc/kernel/pci_impl.h
index 918a203..5f68853 100644
--- a/arch/sparc/kernel/pci_impl.h
+++ b/arch/sparc/kernel/pci_impl.h
@@ -88,7 +88,7 @@  struct pci_pbm_info {
 	int				chip_revision;
 
 	/* Name used for top-level resources. */
-	char				*name;
+	const char			*name;
 
 	/* OBP specific information. */
 	struct platform_device		*op;
diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index 91a375f..4aded3b 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -198,10 +198,10 @@  static unsigned long unflatten_dt_node(struct boot_param_header *blob,
 	np = unflatten_dt_alloc(&mem, sizeof(struct device_node) + allocl,
 				__alignof__(struct device_node));
 	if (allnextpp) {
+		char *fn;
 		memset(np, 0, sizeof(*np));
-		np->full_name = ((char *)np) + sizeof(struct device_node);
+		np->full_name = fn = ((char *)np) + sizeof(*np);
 		if (new_format) {
-			char *fn = np->full_name;
 			/* rebuild full path for new format */
 			if (dad && dad->parent) {
 				strcpy(fn, dad->full_name);
@@ -215,9 +215,9 @@  static unsigned long unflatten_dt_node(struct boot_param_header *blob,
 				fn += strlen(fn);
 			}
 			*(fn++) = '/';
-			memcpy(fn, pathp, l);
-		} else
-			memcpy(np->full_name, pathp, l);
+		}
+		memcpy(fn, pathp, l);
+
 		prev_pp = &np->properties;
 		**allnextpp = np;
 		*allnextpp = &np->allnext;
diff --git a/include/linux/of.h b/include/linux/of.h
index b4e50d5..857dde9 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -46,7 +46,7 @@  struct device_node {
 	const char *name;
 	const char *type;
 	phandle phandle;
-	char	*full_name;
+	const char *full_name;
 
 	struct	property *properties;
 	struct	property *deadprops;	/* removed properties */
@@ -60,7 +60,7 @@  struct device_node {
 	unsigned long _flags;
 	void	*data;
 #if defined(CONFIG_SPARC)
-	char	*path_component_name;
+	const char *path_component_name;
 	unsigned int unique_id;
 	struct of_irq_controller *irq_trans;
 #endif