* [PATCH 1/2] drivers/of: Add check for null property in of_remove_property()
@ 2016-04-28 5:34 ` Suraj Jitindar Singh
0 siblings, 0 replies; 20+ messages in thread
From: Suraj Jitindar Singh @ 2016-04-28 5:34 UTC (permalink / raw)
To: devicetree-u79uwXL29TY76Z2rM5mHXA, linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ
Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
frowand.list-Re5JQEeQqe8AvxtiuMwx3w,
grant.likely-QSEj5FYQhm4dnm+yROfE0A,
benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r,
paulus-eUNUBHrolfbYtjvyW6yDsg, mpe-Gsx/Oe8HsFggBc27wqDAHg,
Suraj Jitindar Singh
The validity of the property input argument to of_remove_property() is
never checked within the function and thus it is possible to pass a null
value. It happens that this will be picked up in __of_remove_property()
as no matching property of the device node will be found and thus an
error will be returned, however once again there is no explicit check
for a null value. By the time this is detected 2 locks have already been
acquired which is completely unnecessary if the property to remove is
null.
Add an explicit check in the function of_remove_property() for a null
property value and return -ENODEV in this case, this is consistent with
what the previous return value would have been when the null value was
not detected and passed to __of_remove_property().
By moving an explicit check for the property paramenter into the
of_remove_property() function, this will remove the need to perform this
check in calling code before invocation of the of_remove_property()
function.
Signed-off-by: Suraj Jitindar Singh <sjitindarsingh-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
drivers/of/base.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index b299de2..64018eb 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -1777,6 +1777,9 @@ int of_remove_property(struct device_node *np, struct property *prop)
unsigned long flags;
int rc;
+ if (!prop)
+ return -ENODEV;
+
mutex_lock(&of_mutex);
raw_spin_lock_irqsave(&devtree_lock, flags);
--
2.5.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 1/2] drivers/of: Add check for null property in of_remove_property()
@ 2016-04-28 5:34 ` Suraj Jitindar Singh
0 siblings, 0 replies; 20+ messages in thread
From: Suraj Jitindar Singh @ 2016-04-28 5:34 UTC (permalink / raw)
To: devicetree, linuxppc-dev
Cc: robh+dt, frowand.list, grant.likely, benh, paulus, mpe,
Suraj Jitindar Singh
The validity of the property input argument to of_remove_property() is
never checked within the function and thus it is possible to pass a null
value. It happens that this will be picked up in __of_remove_property()
as no matching property of the device node will be found and thus an
error will be returned, however once again there is no explicit check
for a null value. By the time this is detected 2 locks have already been
acquired which is completely unnecessary if the property to remove is
null.
Add an explicit check in the function of_remove_property() for a null
property value and return -ENODEV in this case, this is consistent with
what the previous return value would have been when the null value was
not detected and passed to __of_remove_property().
By moving an explicit check for the property paramenter into the
of_remove_property() function, this will remove the need to perform this
check in calling code before invocation of the of_remove_property()
function.
Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
---
drivers/of/base.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index b299de2..64018eb 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -1777,6 +1777,9 @@ int of_remove_property(struct device_node *np, struct property *prop)
unsigned long flags;
int rc;
+ if (!prop)
+ return -ENODEV;
+
mutex_lock(&of_mutex);
raw_spin_lock_irqsave(&devtree_lock, flags);
--
2.5.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 2/2] powerpc: Update of_remove_property() call sites to remove null checking
2016-04-28 5:34 ` Suraj Jitindar Singh
@ 2016-04-28 5:34 ` Suraj Jitindar Singh
-1 siblings, 0 replies; 20+ messages in thread
From: Suraj Jitindar Singh @ 2016-04-28 5:34 UTC (permalink / raw)
To: devicetree-u79uwXL29TY76Z2rM5mHXA, linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ
Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
frowand.list-Re5JQEeQqe8AvxtiuMwx3w,
grant.likely-QSEj5FYQhm4dnm+yROfE0A,
benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r,
paulus-eUNUBHrolfbYtjvyW6yDsg, mpe-Gsx/Oe8HsFggBc27wqDAHg,
Suraj Jitindar Singh
After obtaining a property from of_find_property() and before calling
of_remove_property() most code checks to ensure that the property
returned from of_find_property() is not null. The previous patch
moved this check to the start of the function of_remove_property() in
order to avoid the case where this check isn't done and a null value is
passed. This ensures the check is always conducted before taking locks
and attempting to remove the property. Thus it is no longer necessary
to perform a check for null values before invoking of_remove_property().
Update of_remove_property() call sites in order to remove redundant
checking for null property value as check is now performed within the
of_remove_property function().
Signed-off-by: Suraj Jitindar Singh <sjitindarsingh-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
arch/powerpc/kernel/machine_kexec.c | 19 ++++++-------------
arch/powerpc/kernel/machine_kexec_64.c | 11 ++++-------
arch/powerpc/platforms/pseries/mobility.c | 4 ++--
arch/powerpc/platforms/pseries/reconfig.c | 5 +----
4 files changed, 13 insertions(+), 26 deletions(-)
diff --git a/arch/powerpc/kernel/machine_kexec.c b/arch/powerpc/kernel/machine_kexec.c
index 015ae55..55744a8 100644
--- a/arch/powerpc/kernel/machine_kexec.c
+++ b/arch/powerpc/kernel/machine_kexec.c
@@ -228,17 +228,12 @@ static struct property memory_limit_prop = {
static void __init export_crashk_values(struct device_node *node)
{
- struct property *prop;
-
/* There might be existing crash kernel properties, but we can't
* be sure what's in them, so remove them. */
- prop = of_find_property(node, "linux,crashkernel-base", NULL);
- if (prop)
- of_remove_property(node, prop);
-
- prop = of_find_property(node, "linux,crashkernel-size", NULL);
- if (prop)
- of_remove_property(node, prop);
+ of_remove_property(node, of_find_property(node,
+ "linux,crashkernel-base", NULL));
+ of_remove_property(node, of_find_property(node,
+ "linux,crashkernel-size", NULL));
if (crashk_res.start != 0) {
crashk_base = cpu_to_be_ulong(crashk_res.start),
@@ -258,16 +253,14 @@ static void __init export_crashk_values(struct device_node *node)
static int __init kexec_setup(void)
{
struct device_node *node;
- struct property *prop;
node = of_find_node_by_path("/chosen");
if (!node)
return -ENOENT;
/* remove any stale properties so ours can be found */
- prop = of_find_property(node, kernel_end_prop.name, NULL);
- if (prop)
- of_remove_property(node, prop);
+ of_remove_property(node, of_find_property(node, kernel_end_prop.name,
+ NULL));
/* information needed by userspace when using default_machine_kexec */
kernel_end = cpu_to_be_ulong(__pa(_end));
diff --git a/arch/powerpc/kernel/machine_kexec_64.c b/arch/powerpc/kernel/machine_kexec_64.c
index 0fbd75d..2608192 100644
--- a/arch/powerpc/kernel/machine_kexec_64.c
+++ b/arch/powerpc/kernel/machine_kexec_64.c
@@ -401,7 +401,6 @@ static struct property htab_size_prop = {
static int __init export_htab_values(void)
{
struct device_node *node;
- struct property *prop;
/* On machines with no htab htab_address is NULL */
if (!htab_address)
@@ -412,12 +411,10 @@ static int __init export_htab_values(void)
return -ENODEV;
/* remove any stale propertys so ours can be found */
- prop = of_find_property(node, htab_base_prop.name, NULL);
- if (prop)
- of_remove_property(node, prop);
- prop = of_find_property(node, htab_size_prop.name, NULL);
- if (prop)
- of_remove_property(node, prop);
+ of_remove_property(node, of_find_property(node, htab_base_prop.name,
+ NULL));
+ of_remove_property(node, of_find_property(node, htab_size_prop.name,
+ NULL));
htab_base = cpu_to_be64(__pa(htab_address));
of_add_property(node, &htab_base_prop);
diff --git a/arch/powerpc/platforms/pseries/mobility.c b/arch/powerpc/platforms/pseries/mobility.c
index ceb18d3..a560a98 100644
--- a/arch/powerpc/platforms/pseries/mobility.c
+++ b/arch/powerpc/platforms/pseries/mobility.c
@@ -191,8 +191,8 @@ static int update_dt_node(__be32 phandle, s32 scope)
break;
case 0x80000000:
- prop = of_find_property(dn, prop_name, NULL);
- of_remove_property(dn, prop);
+ of_remove_property(dn, of_find_property(dn,
+ prop_name, NULL));
prop = NULL;
break;
diff --git a/arch/powerpc/platforms/pseries/reconfig.c b/arch/powerpc/platforms/pseries/reconfig.c
index 7c7fcc0..cc66c49 100644
--- a/arch/powerpc/platforms/pseries/reconfig.c
+++ b/arch/powerpc/platforms/pseries/reconfig.c
@@ -303,7 +303,6 @@ static int do_remove_property(char *buf, size_t bufsize)
{
struct device_node *np;
char *tmp;
- struct property *prop;
buf = parse_node(buf, bufsize, &np);
if (!np)
@@ -316,9 +315,7 @@ static int do_remove_property(char *buf, size_t bufsize)
if (strlen(buf) == 0)
return -EINVAL;
- prop = of_find_property(np, buf, NULL);
-
- return of_remove_property(np, prop);
+ return of_remove_property(np, of_find_property(np, buf, NULL));
}
static int do_update_property(char *buf, size_t bufsize)
--
2.5.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 2/2] powerpc: Update of_remove_property() call sites to remove null checking
@ 2016-04-28 5:34 ` Suraj Jitindar Singh
0 siblings, 0 replies; 20+ messages in thread
From: Suraj Jitindar Singh @ 2016-04-28 5:34 UTC (permalink / raw)
To: devicetree, linuxppc-dev
Cc: robh+dt, frowand.list, grant.likely, benh, paulus, mpe,
Suraj Jitindar Singh
After obtaining a property from of_find_property() and before calling
of_remove_property() most code checks to ensure that the property
returned from of_find_property() is not null. The previous patch
moved this check to the start of the function of_remove_property() in
order to avoid the case where this check isn't done and a null value is
passed. This ensures the check is always conducted before taking locks
and attempting to remove the property. Thus it is no longer necessary
to perform a check for null values before invoking of_remove_property().
Update of_remove_property() call sites in order to remove redundant
checking for null property value as check is now performed within the
of_remove_property function().
Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
---
arch/powerpc/kernel/machine_kexec.c | 19 ++++++-------------
arch/powerpc/kernel/machine_kexec_64.c | 11 ++++-------
arch/powerpc/platforms/pseries/mobility.c | 4 ++--
arch/powerpc/platforms/pseries/reconfig.c | 5 +----
4 files changed, 13 insertions(+), 26 deletions(-)
diff --git a/arch/powerpc/kernel/machine_kexec.c b/arch/powerpc/kernel/machine_kexec.c
index 015ae55..55744a8 100644
--- a/arch/powerpc/kernel/machine_kexec.c
+++ b/arch/powerpc/kernel/machine_kexec.c
@@ -228,17 +228,12 @@ static struct property memory_limit_prop = {
static void __init export_crashk_values(struct device_node *node)
{
- struct property *prop;
-
/* There might be existing crash kernel properties, but we can't
* be sure what's in them, so remove them. */
- prop = of_find_property(node, "linux,crashkernel-base", NULL);
- if (prop)
- of_remove_property(node, prop);
-
- prop = of_find_property(node, "linux,crashkernel-size", NULL);
- if (prop)
- of_remove_property(node, prop);
+ of_remove_property(node, of_find_property(node,
+ "linux,crashkernel-base", NULL));
+ of_remove_property(node, of_find_property(node,
+ "linux,crashkernel-size", NULL));
if (crashk_res.start != 0) {
crashk_base = cpu_to_be_ulong(crashk_res.start),
@@ -258,16 +253,14 @@ static void __init export_crashk_values(struct device_node *node)
static int __init kexec_setup(void)
{
struct device_node *node;
- struct property *prop;
node = of_find_node_by_path("/chosen");
if (!node)
return -ENOENT;
/* remove any stale properties so ours can be found */
- prop = of_find_property(node, kernel_end_prop.name, NULL);
- if (prop)
- of_remove_property(node, prop);
+ of_remove_property(node, of_find_property(node, kernel_end_prop.name,
+ NULL));
/* information needed by userspace when using default_machine_kexec */
kernel_end = cpu_to_be_ulong(__pa(_end));
diff --git a/arch/powerpc/kernel/machine_kexec_64.c b/arch/powerpc/kernel/machine_kexec_64.c
index 0fbd75d..2608192 100644
--- a/arch/powerpc/kernel/machine_kexec_64.c
+++ b/arch/powerpc/kernel/machine_kexec_64.c
@@ -401,7 +401,6 @@ static struct property htab_size_prop = {
static int __init export_htab_values(void)
{
struct device_node *node;
- struct property *prop;
/* On machines with no htab htab_address is NULL */
if (!htab_address)
@@ -412,12 +411,10 @@ static int __init export_htab_values(void)
return -ENODEV;
/* remove any stale propertys so ours can be found */
- prop = of_find_property(node, htab_base_prop.name, NULL);
- if (prop)
- of_remove_property(node, prop);
- prop = of_find_property(node, htab_size_prop.name, NULL);
- if (prop)
- of_remove_property(node, prop);
+ of_remove_property(node, of_find_property(node, htab_base_prop.name,
+ NULL));
+ of_remove_property(node, of_find_property(node, htab_size_prop.name,
+ NULL));
htab_base = cpu_to_be64(__pa(htab_address));
of_add_property(node, &htab_base_prop);
diff --git a/arch/powerpc/platforms/pseries/mobility.c b/arch/powerpc/platforms/pseries/mobility.c
index ceb18d3..a560a98 100644
--- a/arch/powerpc/platforms/pseries/mobility.c
+++ b/arch/powerpc/platforms/pseries/mobility.c
@@ -191,8 +191,8 @@ static int update_dt_node(__be32 phandle, s32 scope)
break;
case 0x80000000:
- prop = of_find_property(dn, prop_name, NULL);
- of_remove_property(dn, prop);
+ of_remove_property(dn, of_find_property(dn,
+ prop_name, NULL));
prop = NULL;
break;
diff --git a/arch/powerpc/platforms/pseries/reconfig.c b/arch/powerpc/platforms/pseries/reconfig.c
index 7c7fcc0..cc66c49 100644
--- a/arch/powerpc/platforms/pseries/reconfig.c
+++ b/arch/powerpc/platforms/pseries/reconfig.c
@@ -303,7 +303,6 @@ static int do_remove_property(char *buf, size_t bufsize)
{
struct device_node *np;
char *tmp;
- struct property *prop;
buf = parse_node(buf, bufsize, &np);
if (!np)
@@ -316,9 +315,7 @@ static int do_remove_property(char *buf, size_t bufsize)
if (strlen(buf) == 0)
return -EINVAL;
- prop = of_find_property(np, buf, NULL);
-
- return of_remove_property(np, prop);
+ return of_remove_property(np, of_find_property(np, buf, NULL));
}
static int do_update_property(char *buf, size_t bufsize)
--
2.5.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] drivers/of: Add check for null property in of_remove_property()
2016-04-28 5:34 ` Suraj Jitindar Singh
@ 2016-05-03 13:06 ` Rob Herring
-1 siblings, 0 replies; 20+ messages in thread
From: Rob Herring @ 2016-05-03 13:06 UTC (permalink / raw)
To: Suraj Jitindar Singh
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, linuxppc-dev, Frank Rowand,
Grant Likely, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman
On Thu, Apr 28, 2016 at 12:34 AM, Suraj Jitindar Singh
<sjitindarsingh-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> The validity of the property input argument to of_remove_property() is
> never checked within the function and thus it is possible to pass a null
> value. It happens that this will be picked up in __of_remove_property()
> as no matching property of the device node will be found and thus an
> error will be returned, however once again there is no explicit check
> for a null value. By the time this is detected 2 locks have already been
> acquired which is completely unnecessary if the property to remove is
> null.
>
> Add an explicit check in the function of_remove_property() for a null
> property value and return -ENODEV in this case, this is consistent with
> what the previous return value would have been when the null value was
> not detected and passed to __of_remove_property().
>
> By moving an explicit check for the property paramenter into the
> of_remove_property() function, this will remove the need to perform this
> check in calling code before invocation of the of_remove_property()
> function.
>
> Signed-off-by: Suraj Jitindar Singh <sjitindarsingh-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
For both patches:
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] drivers/of: Add check for null property in of_remove_property()
@ 2016-05-03 13:06 ` Rob Herring
0 siblings, 0 replies; 20+ messages in thread
From: Rob Herring @ 2016-05-03 13:06 UTC (permalink / raw)
To: Suraj Jitindar Singh
Cc: devicetree, linuxppc-dev, Frank Rowand, Grant Likely,
Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
On Thu, Apr 28, 2016 at 12:34 AM, Suraj Jitindar Singh
<sjitindarsingh@gmail.com> wrote:
> The validity of the property input argument to of_remove_property() is
> never checked within the function and thus it is possible to pass a null
> value. It happens that this will be picked up in __of_remove_property()
> as no matching property of the device node will be found and thus an
> error will be returned, however once again there is no explicit check
> for a null value. By the time this is detected 2 locks have already been
> acquired which is completely unnecessary if the property to remove is
> null.
>
> Add an explicit check in the function of_remove_property() for a null
> property value and return -ENODEV in this case, this is consistent with
> what the previous return value would have been when the null value was
> not detected and passed to __of_remove_property().
>
> By moving an explicit check for the property paramenter into the
> of_remove_property() function, this will remove the need to perform this
> check in calling code before invocation of the of_remove_property()
> function.
>
> Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
For both patches:
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] powerpc: Update of_remove_property() call sites to remove null checking
2016-04-28 5:34 ` Suraj Jitindar Singh
@ 2016-05-03 22:32 ` Tyrel Datwyler
-1 siblings, 0 replies; 20+ messages in thread
From: Tyrel Datwyler @ 2016-05-03 22:32 UTC (permalink / raw)
To: Suraj Jitindar Singh, devicetree-u79uwXL29TY76Z2rM5mHXA,
linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ
Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, paulus-eUNUBHrolfbYtjvyW6yDsg,
grant.likely-QSEj5FYQhm4dnm+yROfE0A,
frowand.list-Re5JQEeQqe8AvxtiuMwx3w
On 04/27/2016 10:34 PM, Suraj Jitindar Singh wrote:
> After obtaining a property from of_find_property() and before calling
> of_remove_property() most code checks to ensure that the property
> returned from of_find_property() is not null. The previous patch
> moved this check to the start of the function of_remove_property() in
> order to avoid the case where this check isn't done and a null value is
> passed. This ensures the check is always conducted before taking locks
> and attempting to remove the property. Thus it is no longer necessary
> to perform a check for null values before invoking of_remove_property().
>
> Update of_remove_property() call sites in order to remove redundant
> checking for null property value as check is now performed within the
> of_remove_property function().
>
> Signed-off-by: Suraj Jitindar Singh <sjitindarsingh-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
> arch/powerpc/kernel/machine_kexec.c | 19 ++++++-------------
> arch/powerpc/kernel/machine_kexec_64.c | 11 ++++-------
> arch/powerpc/platforms/pseries/mobility.c | 4 ++--
> arch/powerpc/platforms/pseries/reconfig.c | 5 +----
> 4 files changed, 13 insertions(+), 26 deletions(-)
>
> diff --git a/arch/powerpc/kernel/machine_kexec.c b/arch/powerpc/kernel/machine_kexec.c
> index 015ae55..55744a8 100644
> --- a/arch/powerpc/kernel/machine_kexec.c
> +++ b/arch/powerpc/kernel/machine_kexec.c
> @@ -228,17 +228,12 @@ static struct property memory_limit_prop = {
>
> static void __init export_crashk_values(struct device_node *node)
> {
> - struct property *prop;
> -
> /* There might be existing crash kernel properties, but we can't
> * be sure what's in them, so remove them. */
> - prop = of_find_property(node, "linux,crashkernel-base", NULL);
> - if (prop)
> - of_remove_property(node, prop);
> -
> - prop = of_find_property(node, "linux,crashkernel-size", NULL);
> - if (prop)
> - of_remove_property(node, prop);
> + of_remove_property(node, of_find_property(node,
> + "linux,crashkernel-base", NULL));
> + of_remove_property(node, of_find_property(node,
> + "linux,crashkernel-size", NULL));
>
> if (crashk_res.start != 0) {
> crashk_base = cpu_to_be_ulong(crashk_res.start),
> @@ -258,16 +253,14 @@ static void __init export_crashk_values(struct device_node *node)
> static int __init kexec_setup(void)
> {
> struct device_node *node;
> - struct property *prop;
>
> node = of_find_node_by_path("/chosen");
> if (!node)
> return -ENOENT;
>
> /* remove any stale properties so ours can be found */
> - prop = of_find_property(node, kernel_end_prop.name, NULL);
> - if (prop)
> - of_remove_property(node, prop);
> + of_remove_property(node, of_find_property(node, kernel_end_prop.name,
> + NULL));
>
> /* information needed by userspace when using default_machine_kexec */
> kernel_end = cpu_to_be_ulong(__pa(_end));
> diff --git a/arch/powerpc/kernel/machine_kexec_64.c b/arch/powerpc/kernel/machine_kexec_64.c
> index 0fbd75d..2608192 100644
> --- a/arch/powerpc/kernel/machine_kexec_64.c
> +++ b/arch/powerpc/kernel/machine_kexec_64.c
> @@ -401,7 +401,6 @@ static struct property htab_size_prop = {
> static int __init export_htab_values(void)
> {
> struct device_node *node;
> - struct property *prop;
>
> /* On machines with no htab htab_address is NULL */
> if (!htab_address)
> @@ -412,12 +411,10 @@ static int __init export_htab_values(void)
> return -ENODEV;
>
> /* remove any stale propertys so ours can be found */
> - prop = of_find_property(node, htab_base_prop.name, NULL);
> - if (prop)
> - of_remove_property(node, prop);
> - prop = of_find_property(node, htab_size_prop.name, NULL);
> - if (prop)
> - of_remove_property(node, prop);
> + of_remove_property(node, of_find_property(node, htab_base_prop.name,
> + NULL));
> + of_remove_property(node, of_find_property(node, htab_size_prop.name,
> + NULL));
>
> htab_base = cpu_to_be64(__pa(htab_address));
> of_add_property(node, &htab_base_prop);
> diff --git a/arch/powerpc/platforms/pseries/mobility.c b/arch/powerpc/platforms/pseries/mobility.c
> index ceb18d3..a560a98 100644
> --- a/arch/powerpc/platforms/pseries/mobility.c
> +++ b/arch/powerpc/platforms/pseries/mobility.c
> @@ -191,8 +191,8 @@ static int update_dt_node(__be32 phandle, s32 scope)
> break;
>
> case 0x80000000:
> - prop = of_find_property(dn, prop_name, NULL);
> - of_remove_property(dn, prop);
> + of_remove_property(dn, of_find_property(dn,
> + prop_name, NULL));
> prop = NULL;
> break;
>
You haven't removed a NULL check here, as suggested by the changelog,
but instead made a cosmetic change to the code that still leaves behind
a now unnecessary "prop = NULL;" to bit rot.
> diff --git a/arch/powerpc/platforms/pseries/reconfig.c b/arch/powerpc/platforms/pseries/reconfig.c
> index 7c7fcc0..cc66c49 100644
> --- a/arch/powerpc/platforms/pseries/reconfig.c
> +++ b/arch/powerpc/platforms/pseries/reconfig.c
> @@ -303,7 +303,6 @@ static int do_remove_property(char *buf, size_t bufsize)
> {
> struct device_node *np;
> char *tmp;
> - struct property *prop;
> buf = parse_node(buf, bufsize, &np);
>
> if (!np)
> @@ -316,9 +315,7 @@ static int do_remove_property(char *buf, size_t bufsize)
> if (strlen(buf) == 0)
> return -EINVAL;
>
> - prop = of_find_property(np, buf, NULL);
> -
> - return of_remove_property(np, prop);
> + return of_remove_property(np, of_find_property(np, buf, NULL));
> }
Again, you are not removing a NULL check as suggested by the changelog.
-Tyrel
>
> static int do_update_property(char *buf, size_t bufsize)
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] powerpc: Update of_remove_property() call sites to remove null checking
@ 2016-05-03 22:32 ` Tyrel Datwyler
0 siblings, 0 replies; 20+ messages in thread
From: Tyrel Datwyler @ 2016-05-03 22:32 UTC (permalink / raw)
To: Suraj Jitindar Singh, devicetree, linuxppc-dev
Cc: robh+dt, paulus, grant.likely, frowand.list
On 04/27/2016 10:34 PM, Suraj Jitindar Singh wrote:
> After obtaining a property from of_find_property() and before calling
> of_remove_property() most code checks to ensure that the property
> returned from of_find_property() is not null. The previous patch
> moved this check to the start of the function of_remove_property() in
> order to avoid the case where this check isn't done and a null value is
> passed. This ensures the check is always conducted before taking locks
> and attempting to remove the property. Thus it is no longer necessary
> to perform a check for null values before invoking of_remove_property().
>
> Update of_remove_property() call sites in order to remove redundant
> checking for null property value as check is now performed within the
> of_remove_property function().
>
> Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
> ---
> arch/powerpc/kernel/machine_kexec.c | 19 ++++++-------------
> arch/powerpc/kernel/machine_kexec_64.c | 11 ++++-------
> arch/powerpc/platforms/pseries/mobility.c | 4 ++--
> arch/powerpc/platforms/pseries/reconfig.c | 5 +----
> 4 files changed, 13 insertions(+), 26 deletions(-)
>
> diff --git a/arch/powerpc/kernel/machine_kexec.c b/arch/powerpc/kernel/machine_kexec.c
> index 015ae55..55744a8 100644
> --- a/arch/powerpc/kernel/machine_kexec.c
> +++ b/arch/powerpc/kernel/machine_kexec.c
> @@ -228,17 +228,12 @@ static struct property memory_limit_prop = {
>
> static void __init export_crashk_values(struct device_node *node)
> {
> - struct property *prop;
> -
> /* There might be existing crash kernel properties, but we can't
> * be sure what's in them, so remove them. */
> - prop = of_find_property(node, "linux,crashkernel-base", NULL);
> - if (prop)
> - of_remove_property(node, prop);
> -
> - prop = of_find_property(node, "linux,crashkernel-size", NULL);
> - if (prop)
> - of_remove_property(node, prop);
> + of_remove_property(node, of_find_property(node,
> + "linux,crashkernel-base", NULL));
> + of_remove_property(node, of_find_property(node,
> + "linux,crashkernel-size", NULL));
>
> if (crashk_res.start != 0) {
> crashk_base = cpu_to_be_ulong(crashk_res.start),
> @@ -258,16 +253,14 @@ static void __init export_crashk_values(struct device_node *node)
> static int __init kexec_setup(void)
> {
> struct device_node *node;
> - struct property *prop;
>
> node = of_find_node_by_path("/chosen");
> if (!node)
> return -ENOENT;
>
> /* remove any stale properties so ours can be found */
> - prop = of_find_property(node, kernel_end_prop.name, NULL);
> - if (prop)
> - of_remove_property(node, prop);
> + of_remove_property(node, of_find_property(node, kernel_end_prop.name,
> + NULL));
>
> /* information needed by userspace when using default_machine_kexec */
> kernel_end = cpu_to_be_ulong(__pa(_end));
> diff --git a/arch/powerpc/kernel/machine_kexec_64.c b/arch/powerpc/kernel/machine_kexec_64.c
> index 0fbd75d..2608192 100644
> --- a/arch/powerpc/kernel/machine_kexec_64.c
> +++ b/arch/powerpc/kernel/machine_kexec_64.c
> @@ -401,7 +401,6 @@ static struct property htab_size_prop = {
> static int __init export_htab_values(void)
> {
> struct device_node *node;
> - struct property *prop;
>
> /* On machines with no htab htab_address is NULL */
> if (!htab_address)
> @@ -412,12 +411,10 @@ static int __init export_htab_values(void)
> return -ENODEV;
>
> /* remove any stale propertys so ours can be found */
> - prop = of_find_property(node, htab_base_prop.name, NULL);
> - if (prop)
> - of_remove_property(node, prop);
> - prop = of_find_property(node, htab_size_prop.name, NULL);
> - if (prop)
> - of_remove_property(node, prop);
> + of_remove_property(node, of_find_property(node, htab_base_prop.name,
> + NULL));
> + of_remove_property(node, of_find_property(node, htab_size_prop.name,
> + NULL));
>
> htab_base = cpu_to_be64(__pa(htab_address));
> of_add_property(node, &htab_base_prop);
> diff --git a/arch/powerpc/platforms/pseries/mobility.c b/arch/powerpc/platforms/pseries/mobility.c
> index ceb18d3..a560a98 100644
> --- a/arch/powerpc/platforms/pseries/mobility.c
> +++ b/arch/powerpc/platforms/pseries/mobility.c
> @@ -191,8 +191,8 @@ static int update_dt_node(__be32 phandle, s32 scope)
> break;
>
> case 0x80000000:
> - prop = of_find_property(dn, prop_name, NULL);
> - of_remove_property(dn, prop);
> + of_remove_property(dn, of_find_property(dn,
> + prop_name, NULL));
> prop = NULL;
> break;
>
You haven't removed a NULL check here, as suggested by the changelog,
but instead made a cosmetic change to the code that still leaves behind
a now unnecessary "prop = NULL;" to bit rot.
> diff --git a/arch/powerpc/platforms/pseries/reconfig.c b/arch/powerpc/platforms/pseries/reconfig.c
> index 7c7fcc0..cc66c49 100644
> --- a/arch/powerpc/platforms/pseries/reconfig.c
> +++ b/arch/powerpc/platforms/pseries/reconfig.c
> @@ -303,7 +303,6 @@ static int do_remove_property(char *buf, size_t bufsize)
> {
> struct device_node *np;
> char *tmp;
> - struct property *prop;
> buf = parse_node(buf, bufsize, &np);
>
> if (!np)
> @@ -316,9 +315,7 @@ static int do_remove_property(char *buf, size_t bufsize)
> if (strlen(buf) == 0)
> return -EINVAL;
>
> - prop = of_find_property(np, buf, NULL);
> -
> - return of_remove_property(np, prop);
> + return of_remove_property(np, of_find_property(np, buf, NULL));
> }
Again, you are not removing a NULL check as suggested by the changelog.
-Tyrel
>
> static int do_update_property(char *buf, size_t bufsize)
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [1/2] drivers/of: Add check for null property in of_remove_property()
2016-04-28 5:34 ` Suraj Jitindar Singh
@ 2016-05-04 22:40 ` Michael Ellerman
-1 siblings, 0 replies; 20+ messages in thread
From: Michael Ellerman @ 2016-05-04 22:40 UTC (permalink / raw)
To: devicetree-u79uwXL29TY76Z2rM5mHXA, linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ
Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, paulus-eUNUBHrolfbYtjvyW6yDsg,
Suraj Jitindar Singh, grant.likely-QSEj5FYQhm4dnm+yROfE0A,
frowand.list-Re5JQEeQqe8AvxtiuMwx3w
On Thu, 2016-28-04 at 05:34:54 UTC, Suraj Jitindar Singh wrote:
> The validity of the property input argument to of_remove_property() is
> never checked within the function and thus it is possible to pass a null
> value. It happens that this will be picked up in __of_remove_property()
> as no matching property of the device node will be found and thus an
> error will be returned, however once again there is no explicit check
> for a null value. By the time this is detected 2 locks have already been
> acquired which is completely unnecessary if the property to remove is
> null.
>
> Add an explicit check in the function of_remove_property() for a null
> property value and return -ENODEV in this case, this is consistent with
> what the previous return value would have been when the null value was
> not detected and passed to __of_remove_property().
>
> By moving an explicit check for the property paramenter into the
> of_remove_property() function, this will remove the need to perform this
> check in calling code before invocation of the of_remove_property()
> function.
>
> Signed-off-by: Suraj Jitindar Singh <sjitindarsingh-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/1c173cb23486f540ea08a5050a
cheers
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [1/2] drivers/of: Add check for null property in of_remove_property()
@ 2016-05-04 22:40 ` Michael Ellerman
0 siblings, 0 replies; 20+ messages in thread
From: Michael Ellerman @ 2016-05-04 22:40 UTC (permalink / raw)
To: Suraj Jitindar Singh, devicetree, linuxppc-dev
Cc: robh+dt, paulus, Suraj Jitindar Singh, grant.likely, frowand.list
On Thu, 2016-28-04 at 05:34:54 UTC, Suraj Jitindar Singh wrote:
> The validity of the property input argument to of_remove_property() is
> never checked within the function and thus it is possible to pass a null
> value. It happens that this will be picked up in __of_remove_property()
> as no matching property of the device node will be found and thus an
> error will be returned, however once again there is no explicit check
> for a null value. By the time this is detected 2 locks have already been
> acquired which is completely unnecessary if the property to remove is
> null.
>
> Add an explicit check in the function of_remove_property() for a null
> property value and return -ENODEV in this case, this is consistent with
> what the previous return value would have been when the null value was
> not detected and passed to __of_remove_property().
>
> By moving an explicit check for the property paramenter into the
> of_remove_property() function, this will remove the need to perform this
> check in calling code before invocation of the of_remove_property()
> function.
>
> Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
> Acked-by: Rob Herring <robh@kernel.org>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/1c173cb23486f540ea08a5050a
cheers
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [2/2] powerpc: Update of_remove_property() call sites to remove null checking
2016-04-28 5:34 ` Suraj Jitindar Singh
@ 2016-05-04 22:40 ` Michael Ellerman
-1 siblings, 0 replies; 20+ messages in thread
From: Michael Ellerman @ 2016-05-04 22:40 UTC (permalink / raw)
To: devicetree-u79uwXL29TY76Z2rM5mHXA, linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ
Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, paulus-eUNUBHrolfbYtjvyW6yDsg,
Suraj Jitindar Singh, grant.likely-QSEj5FYQhm4dnm+yROfE0A,
frowand.list-Re5JQEeQqe8AvxtiuMwx3w
On Thu, 2016-28-04 at 05:34:55 UTC, Suraj Jitindar Singh wrote:
> After obtaining a property from of_find_property() and before calling
> of_remove_property() most code checks to ensure that the property
> returned from of_find_property() is not null. The previous patch
> moved this check to the start of the function of_remove_property() in
> order to avoid the case where this check isn't done and a null value is
> passed. This ensures the check is always conducted before taking locks
> and attempting to remove the property. Thus it is no longer necessary
> to perform a check for null values before invoking of_remove_property().
>
> Update of_remove_property() call sites in order to remove redundant
> checking for null property value as check is now performed within the
> of_remove_property function().
>
> Signed-off-by: Suraj Jitindar Singh <sjitindarsingh-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/676669b666574a23f7bd62870c
cheers
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [2/2] powerpc: Update of_remove_property() call sites to remove null checking
@ 2016-05-04 22:40 ` Michael Ellerman
0 siblings, 0 replies; 20+ messages in thread
From: Michael Ellerman @ 2016-05-04 22:40 UTC (permalink / raw)
To: Suraj Jitindar Singh, devicetree, linuxppc-dev
Cc: robh+dt, paulus, Suraj Jitindar Singh, grant.likely, frowand.list
On Thu, 2016-28-04 at 05:34:55 UTC, Suraj Jitindar Singh wrote:
> After obtaining a property from of_find_property() and before calling
> of_remove_property() most code checks to ensure that the property
> returned from of_find_property() is not null. The previous patch
> moved this check to the start of the function of_remove_property() in
> order to avoid the case where this check isn't done and a null value is
> passed. This ensures the check is always conducted before taking locks
> and attempting to remove the property. Thus it is no longer necessary
> to perform a check for null values before invoking of_remove_property().
>
> Update of_remove_property() call sites in order to remove redundant
> checking for null property value as check is now performed within the
> of_remove_property function().
>
> Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/676669b666574a23f7bd62870c
cheers
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] powerpc: Update of_remove_property() call sites to remove null checking
2016-05-03 22:32 ` Tyrel Datwyler
@ 2016-05-05 6:50 ` Michael Ellerman
-1 siblings, 0 replies; 20+ messages in thread
From: Michael Ellerman @ 2016-05-05 6:50 UTC (permalink / raw)
To: Tyrel Datwyler, Suraj Jitindar Singh,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ
Cc: grant.likely-QSEj5FYQhm4dnm+yROfE0A,
frowand.list-Re5JQEeQqe8AvxtiuMwx3w,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, paulus-eUNUBHrolfbYtjvyW6yDsg
On Tue, 2016-05-03 at 15:32 -0700, Tyrel Datwyler wrote:
> On 04/27/2016 10:34 PM, Suraj Jitindar Singh wrote:
> > diff --git a/arch/powerpc/platforms/pseries/mobility.c b/arch/powerpc/platforms/pseries/mobility.c
> > index ceb18d3..a560a98 100644
> > --- a/arch/powerpc/platforms/pseries/mobility.c
> > +++ b/arch/powerpc/platforms/pseries/mobility.c
> > @@ -191,8 +191,8 @@ static int update_dt_node(__be32 phandle, s32 scope)
> > break;
> >
> > case 0x80000000:
> > - prop = of_find_property(dn, prop_name, NULL);
> > - of_remove_property(dn, prop);
> > + of_remove_property(dn, of_find_property(dn,
> > + prop_name, NULL));
> > prop = NULL;
> > break;
> >
>
> You haven't removed a NULL check here, as suggested by the changelog,
> but instead made a cosmetic change to the code that still leaves behind
> a now unnecessary "prop = NULL;" to bit rot.
Yeah I think you're right. Though it's not very clear how prop is used in that
function.
Please one of you send me an incremental to remove the prop = NULL;
cheers
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] powerpc: Update of_remove_property() call sites to remove null checking
@ 2016-05-05 6:50 ` Michael Ellerman
0 siblings, 0 replies; 20+ messages in thread
From: Michael Ellerman @ 2016-05-05 6:50 UTC (permalink / raw)
To: Tyrel Datwyler, Suraj Jitindar Singh, devicetree, linuxppc-dev
Cc: grant.likely, frowand.list, robh+dt, paulus
On Tue, 2016-05-03 at 15:32 -0700, Tyrel Datwyler wrote:
> On 04/27/2016 10:34 PM, Suraj Jitindar Singh wrote:
> > diff --git a/arch/powerpc/platforms/pseries/mobility.c b/arch/powerpc/platforms/pseries/mobility.c
> > index ceb18d3..a560a98 100644
> > --- a/arch/powerpc/platforms/pseries/mobility.c
> > +++ b/arch/powerpc/platforms/pseries/mobility.c
> > @@ -191,8 +191,8 @@ static int update_dt_node(__be32 phandle, s32 scope)
> > break;
> >
> > case 0x80000000:
> > - prop = of_find_property(dn, prop_name, NULL);
> > - of_remove_property(dn, prop);
> > + of_remove_property(dn, of_find_property(dn,
> > + prop_name, NULL));
> > prop = NULL;
> > break;
> >
>
> You haven't removed a NULL check here, as suggested by the changelog,
> but instead made a cosmetic change to the code that still leaves behind
> a now unnecessary "prop = NULL;" to bit rot.
Yeah I think you're right. Though it's not very clear how prop is used in that
function.
Please one of you send me an incremental to remove the prop = NULL;
cheers
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] powerpc: Update of_remove_property() call sites to remove null checking
2016-05-05 6:50 ` Michael Ellerman
@ 2016-05-06 1:01 ` Suraj Jitindar Singh
-1 siblings, 0 replies; 20+ messages in thread
From: Suraj Jitindar Singh @ 2016-05-06 1:01 UTC (permalink / raw)
To: Michael Ellerman, Tyrel Datwyler, devicetree, linuxppc-dev
Cc: grant.likely, paulus, robh+dt, frowand.list
[-- Attachment #1.1: Type: text/plain, Size: 1383 bytes --]
On 05/05/16 16:50, Michael Ellerman wrote:
> On Tue, 2016-05-03 at 15:32 -0700, Tyrel Datwyler wrote: >> On 04/27/2016 10:34 PM, Suraj Jitindar Singh wrote: >>> diff --git a/arch/powerpc/platforms/pseries/mobility.c b/arch/powerpc/platforms/pseries/mobility.c >>> index ceb18d3..a560a98 100644 >>> --- a/arch/powerpc/platforms/pseries/mobility.c >>> +++ b/arch/powerpc/platforms/pseries/mobility.c >>> @@ -191,8 +191,8 @@ static int update_dt_node(__be32 phandle, s32 scope) >>> break; >>> >>> case 0x80000000: >>> - prop = of_find_property(dn, prop_name, NULL); >>> - of_remove_property(dn, prop); >>> + of_remove_property(dn, of_find_property(dn, >>> + prop_name, NULL)); >>> prop = NULL; >>> break; >>> >> >> You haven't removed a NULL check here, as suggested by the changelog, >> but instead made a cosmetic change to the code that still leaves behind >> a now unnecessary "prop = NULL;" to bit rot. > > Yeah I think you're right. Though it's not
very clear how prop is used in that > function. > > Please one of you send me an incremental to remove the prop = NULL; > > cheers >
I didn't delete the prop = NULL; initially as I didn't fully understand
how it was used in the rest of the function and the effect of deleting
it.
[-- Attachment #1.2: Type: text/html, Size: 1996 bytes --]
[-- Attachment #2: Type: text/plain, Size: 150 bytes --]
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] powerpc: Update of_remove_property() call sites to remove null checking
@ 2016-05-06 1:01 ` Suraj Jitindar Singh
0 siblings, 0 replies; 20+ messages in thread
From: Suraj Jitindar Singh @ 2016-05-06 1:01 UTC (permalink / raw)
To: Michael Ellerman, Tyrel Datwyler, devicetree, linuxppc-dev
Cc: grant.likely, frowand.list, robh+dt, paulus
[-- Attachment #1: Type: text/plain, Size: 1383 bytes --]
On 05/05/16 16:50, Michael Ellerman wrote:
> On Tue, 2016-05-03 at 15:32 -0700, Tyrel Datwyler wrote: >> On 04/27/2016 10:34 PM, Suraj Jitindar Singh wrote: >>> diff --git a/arch/powerpc/platforms/pseries/mobility.c b/arch/powerpc/platforms/pseries/mobility.c >>> index ceb18d3..a560a98 100644 >>> --- a/arch/powerpc/platforms/pseries/mobility.c >>> +++ b/arch/powerpc/platforms/pseries/mobility.c >>> @@ -191,8 +191,8 @@ static int update_dt_node(__be32 phandle, s32 scope) >>> break; >>> >>> case 0x80000000: >>> - prop = of_find_property(dn, prop_name, NULL); >>> - of_remove_property(dn, prop); >>> + of_remove_property(dn, of_find_property(dn, >>> + prop_name, NULL)); >>> prop = NULL; >>> break; >>> >> >> You haven't removed a NULL check here, as suggested by the changelog, >> but instead made a cosmetic change to the code that still leaves behind >> a now unnecessary "prop = NULL;" to bit rot. > > Yeah I think you're right. Though it's not
very clear how prop is used in that > function. > > Please one of you send me an incremental to remove the prop = NULL; > > cheers >
I didn't delete the prop = NULL; initially as I didn't fully understand
how it was used in the rest of the function and the effect of deleting
it.
[-- Attachment #2: Type: text/html, Size: 1950 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] powerpc: Update of_remove_property() call sites to remove null checking
2016-05-05 6:50 ` Michael Ellerman
@ 2016-05-06 3:00 ` Suraj Jitindar Singh
-1 siblings, 0 replies; 20+ messages in thread
From: Suraj Jitindar Singh @ 2016-05-06 3:00 UTC (permalink / raw)
To: Michael Ellerman, Tyrel Datwyler,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ
Cc: grant.likely-QSEj5FYQhm4dnm+yROfE0A,
frowand.list-Re5JQEeQqe8AvxtiuMwx3w,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, paulus-eUNUBHrolfbYtjvyW6yDsg
On 05/05/16 16:50, Michael Ellerman wrote:
> On Tue, 2016-05-03 at 15:32 -0700, Tyrel Datwyler wrote:
>> On 04/27/2016 10:34 PM, Suraj Jitindar Singh wrote:
>>> diff --git a/arch/powerpc/platforms/pseries/mobility.c b/arch/powerpc/platforms/pseries/mobility.c
>>> index ceb18d3..a560a98 100644
>>> --- a/arch/powerpc/platforms/pseries/mobility.c
>>> +++ b/arch/powerpc/platforms/pseries/mobility.c
>>> @@ -191,8 +191,8 @@ static int update_dt_node(__be32 phandle, s32 scope)
>>> break;
>>>
>>> case 0x80000000:
>>> - prop = of_find_property(dn, prop_name, NULL);
>>> - of_remove_property(dn, prop);
>>> + of_remove_property(dn, of_find_property(dn,
>>> + prop_name, NULL));
>>> prop = NULL;
>>> break;
>>>
>> You haven't removed a NULL check here, as suggested by the changelog,
>> but instead made a cosmetic change to the code that still leaves behind
>> a now unnecessary "prop = NULL;" to bit rot.
> Yeah I think you're right. Though it's not very clear how prop is used in that
> function.
>
> Please one of you send me an incremental to remove the prop = NULL;
>
> cheers
>
Resend of previous message due to formatting issues:
I didn't delete the prop = NULL; initially as I didn't fully understand
how it was used in the rest of the function and the effect of deleting
it.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] powerpc: Update of_remove_property() call sites to remove null checking
@ 2016-05-06 3:00 ` Suraj Jitindar Singh
0 siblings, 0 replies; 20+ messages in thread
From: Suraj Jitindar Singh @ 2016-05-06 3:00 UTC (permalink / raw)
To: Michael Ellerman, Tyrel Datwyler, devicetree, linuxppc-dev
Cc: grant.likely, frowand.list, robh+dt, paulus
On 05/05/16 16:50, Michael Ellerman wrote:
> On Tue, 2016-05-03 at 15:32 -0700, Tyrel Datwyler wrote:
>> On 04/27/2016 10:34 PM, Suraj Jitindar Singh wrote:
>>> diff --git a/arch/powerpc/platforms/pseries/mobility.c b/arch/powerpc/platforms/pseries/mobility.c
>>> index ceb18d3..a560a98 100644
>>> --- a/arch/powerpc/platforms/pseries/mobility.c
>>> +++ b/arch/powerpc/platforms/pseries/mobility.c
>>> @@ -191,8 +191,8 @@ static int update_dt_node(__be32 phandle, s32 scope)
>>> break;
>>>
>>> case 0x80000000:
>>> - prop = of_find_property(dn, prop_name, NULL);
>>> - of_remove_property(dn, prop);
>>> + of_remove_property(dn, of_find_property(dn,
>>> + prop_name, NULL));
>>> prop = NULL;
>>> break;
>>>
>> You haven't removed a NULL check here, as suggested by the changelog,
>> but instead made a cosmetic change to the code that still leaves behind
>> a now unnecessary "prop = NULL;" to bit rot.
> Yeah I think you're right. Though it's not very clear how prop is used in that
> function.
>
> Please one of you send me an incremental to remove the prop = NULL;
>
> cheers
>
Resend of previous message due to formatting issues:
I didn't delete the prop = NULL; initially as I didn't fully understand
how it was used in the rest of the function and the effect of deleting
it.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] powerpc: Update of_remove_property() call sites to remove null checking
2016-05-06 3:00 ` Suraj Jitindar Singh
@ 2016-05-09 9:41 ` Michael Ellerman
-1 siblings, 0 replies; 20+ messages in thread
From: Michael Ellerman @ 2016-05-09 9:41 UTC (permalink / raw)
To: Suraj Jitindar Singh, Tyrel Datwyler,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ
Cc: grant.likely-QSEj5FYQhm4dnm+yROfE0A,
frowand.list-Re5JQEeQqe8AvxtiuMwx3w,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, paulus-eUNUBHrolfbYtjvyW6yDsg
On Fri, 2016-05-06 at 13:00 +1000, Suraj Jitindar Singh wrote:
> On 05/05/16 16:50, Michael Ellerman wrote:
> > On Tue, 2016-05-03 at 15:32 -0700, Tyrel Datwyler wrote:
> > > On 04/27/2016 10:34 PM, Suraj Jitindar Singh wrote:
> > > > diff --git a/arch/powerpc/platforms/pseries/mobility.c b/arch/powerpc/platforms/pseries/mobility.c
> > > > index ceb18d3..a560a98 100644
> > > > --- a/arch/powerpc/platforms/pseries/mobility.c
> > > > +++ b/arch/powerpc/platforms/pseries/mobility.c
> > > > @@ -191,8 +191,8 @@ static int update_dt_node(__be32 phandle, s32 scope)
> > > > break;
> > > >
> > > > case 0x80000000:
> > > > - prop = of_find_property(dn, prop_name, NULL);
> > > > - of_remove_property(dn, prop);
> > > > + of_remove_property(dn, of_find_property(dn,
> > > > + prop_name, NULL));
> > > > prop = NULL;
> > > > break;
> > > >
> > > You haven't removed a NULL check here, as suggested by the changelog,
> > > but instead made a cosmetic change to the code that still leaves behind
> > > a now unnecessary "prop = NULL;" to bit rot.
> > Yeah I think you're right. Though it's not very clear how prop is used in that
> > function.
>
> I didn't delete the prop = NULL; initially as I didn't fully understand
> how it was used in the rest of the function and the effect of deleting
> it.
Yeah, it's pretty convoluted. I don't think you can actually prove it's safe to
remove the prop = NULL for arbitrary inputs.
cheers
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] powerpc: Update of_remove_property() call sites to remove null checking
@ 2016-05-09 9:41 ` Michael Ellerman
0 siblings, 0 replies; 20+ messages in thread
From: Michael Ellerman @ 2016-05-09 9:41 UTC (permalink / raw)
To: Suraj Jitindar Singh, Tyrel Datwyler, devicetree, linuxppc-dev
Cc: grant.likely, frowand.list, robh+dt, paulus
On Fri, 2016-05-06 at 13:00 +1000, Suraj Jitindar Singh wrote:
> On 05/05/16 16:50, Michael Ellerman wrote:
> > On Tue, 2016-05-03 at 15:32 -0700, Tyrel Datwyler wrote:
> > > On 04/27/2016 10:34 PM, Suraj Jitindar Singh wrote:
> > > > diff --git a/arch/powerpc/platforms/pseries/mobility.c b/arch/powerpc/platforms/pseries/mobility.c
> > > > index ceb18d3..a560a98 100644
> > > > --- a/arch/powerpc/platforms/pseries/mobility.c
> > > > +++ b/arch/powerpc/platforms/pseries/mobility.c
> > > > @@ -191,8 +191,8 @@ static int update_dt_node(__be32 phandle, s32 scope)
> > > > break;
> > > >
> > > > case 0x80000000:
> > > > - prop = of_find_property(dn, prop_name, NULL);
> > > > - of_remove_property(dn, prop);
> > > > + of_remove_property(dn, of_find_property(dn,
> > > > + prop_name, NULL));
> > > > prop = NULL;
> > > > break;
> > > >
> > > You haven't removed a NULL check here, as suggested by the changelog,
> > > but instead made a cosmetic change to the code that still leaves behind
> > > a now unnecessary "prop = NULL;" to bit rot.
> > Yeah I think you're right. Though it's not very clear how prop is used in that
> > function.
>
> I didn't delete the prop = NULL; initially as I didn't fully understand
> how it was used in the rest of the function and the effect of deleting
> it.
Yeah, it's pretty convoluted. I don't think you can actually prove it's safe to
remove the prop = NULL for arbitrary inputs.
cheers
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2016-05-09 9:41 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-28 5:34 [PATCH 1/2] drivers/of: Add check for null property in of_remove_property() Suraj Jitindar Singh
2016-04-28 5:34 ` Suraj Jitindar Singh
[not found] ` <1461821695-19204-1-git-send-email-sjitindarsingh-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2016-04-28 5:34 ` [PATCH 2/2] powerpc: Update of_remove_property() call sites to remove null checking Suraj Jitindar Singh
2016-04-28 5:34 ` Suraj Jitindar Singh
[not found] ` <1461821695-19204-2-git-send-email-sjitindarsingh-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2016-05-03 22:32 ` Tyrel Datwyler
2016-05-03 22:32 ` Tyrel Datwyler
[not found] ` <57292702.5070507-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
2016-05-05 6:50 ` Michael Ellerman
2016-05-05 6:50 ` Michael Ellerman
2016-05-06 1:01 ` Suraj Jitindar Singh
2016-05-06 1:01 ` Suraj Jitindar Singh
[not found] ` <1462431031.12473.3.camel-Gsx/Oe8HsFggBc27wqDAHg@public.gmane.org>
2016-05-06 3:00 ` Suraj Jitindar Singh
2016-05-06 3:00 ` Suraj Jitindar Singh
[not found] ` <572C08DB.9000109-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2016-05-09 9:41 ` Michael Ellerman
2016-05-09 9:41 ` Michael Ellerman
2016-05-04 22:40 ` [2/2] " Michael Ellerman
2016-05-04 22:40 ` Michael Ellerman
2016-05-03 13:06 ` [PATCH 1/2] drivers/of: Add check for null property in of_remove_property() Rob Herring
2016-05-03 13:06 ` Rob Herring
2016-05-04 22:40 ` [1/2] " Michael Ellerman
2016-05-04 22:40 ` Michael Ellerman
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.