2012-11-16 14:20:34

by Grant Likely

[permalink] [raw]
Subject: [PATCH v2] drivers/of: Constify device_node->name and ->path_component_name

Neither of these should ever be changed once set. Make them const and
fix up the users that try to modify it in-place. In one case
kmalloc+memcpy is replaced with kstrdup() to avoid modifying the string.

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

Signed-off-by: Grant Likely <[email protected]>
Cc: Benjamin Herrenschmidt <[email protected]>
Cc: "David S. Miller" <[email protected]>
Cc: Julian Calaby <[email protected]>
---

v2 - fill out more information in the commit text.

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(-)

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 c2b08dc..c8be326 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -199,10 +199,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);
@@ -216,9 +216,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
--
1.7.10.4


2012-11-16 17:34:03

by David Miller

[permalink] [raw]
Subject: Re: [PATCH v2] drivers/of: Constify device_node->name and ->path_component_name

From: Grant Likely <[email protected]>
Date: Fri, 16 Nov 2012 14:20:25 +0000

> Neither of these should ever be changed once set. Make them const and
> fix up the users that try to modify it in-place. In one case
> kmalloc+memcpy is replaced with kstrdup() to avoid modifying the string.
>
> Build tested with defconfigs on ARM, PowerPC, Sparc, MIPS, x86 among
> others.
>
> Signed-off-by: Grant Likely <[email protected]>

Acked-by: David S. Miller <[email protected]>

2012-11-27 23:46:09

by Michael Neuling

[permalink] [raw]
Subject: Re: [PATCH v2] drivers/of: Constify device_node->name and ->path_component_name

> Neither of these should ever be changed once set. Make them const and
> fix up the users that try to modify it in-place. In one case
> kmalloc+memcpy is replaced with kstrdup() to avoid modifying the string.
>
> Build tested with defconfigs on ARM, PowerPC, Sparc, MIPS, x86 among
> others.

Grant,

This breaks powerpc chroma_defconfig in next-20121127 with:

arch/powerpc/sysdev/scom.c:160:17: error: assignment discards 'const' qualifier from pointer target type [-Werror]

The following fixes it. The change is to generic code, so I'm not sure
it's the right fix as it may break other configs/archs.

diff --git a/include/linux/debugfs.h b/include/linux/debugfs.h
index 66c434f..77f64e4 100644
--- a/include/linux/debugfs.h
+++ b/include/linux/debugfs.h
@@ -23,7 +23,7 @@
struct file_operations;

struct debugfs_blob_wrapper {
- void *data;
+ const void *data;
unsigned long size;
};

2012-11-29 16:34:21

by Grant Likely

[permalink] [raw]
Subject: Re: [PATCH v2] drivers/of: Constify device_node->name and ->path_component_name

On Wed, 28 Nov 2012 10:46:04 +1100, Michael Neuling <[email protected]> wrote:
> > Neither of these should ever be changed once set. Make them const and
> > fix up the users that try to modify it in-place. In one case
> > kmalloc+memcpy is replaced with kstrdup() to avoid modifying the string.
> >
> > Build tested with defconfigs on ARM, PowerPC, Sparc, MIPS, x86 among
> > others.
>
> Grant,
>
> This breaks powerpc chroma_defconfig in next-20121127 with:
>
> arch/powerpc/sysdev/scom.c:160:17: error: assignment discards 'const' qualifier from pointer target type [-Werror]
>
> The following fixes it. The change is to generic code, so I'm not sure
> it's the right fix as it may break other configs/archs.

That is rather risky. Post it as a patch to core code, but in the mean
time I'll patch the powerpc scom code.

g.