fdt: Add a do_fixup_by_path_string() function

PATCH Superseded Chunhe Lan 5 participants 6 messages Aug 17, 2011 – Aug 29, 2011

Share & tools

Chunhe Lan

author

2011-08-17 06:24 UTC #0 lore raw

PATCH v3 1/2 fdt: Add a do_fixup_by_path_string() function

The do_fixup_by_path_string() will set the specified node's property to the value contained in "status". It would just be a wrapper for do_fixup_by_path() that calls strlen on the argument.

Signed-off-by Chunhe Lan <[email protected]>

---
 include/fdt_support.h |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

include/fdt_support.h +7 -0

index 863024f..1de4a1d 100644
@@ -36,6 +36,13 @@ void do_fixup_by_path(void *fdt, const char *path, const char *prop,
 		      const void *val, int len, int create);
 void do_fixup_by_path_u32(void *fdt, const char *path, const char *prop,
 			  u32 val, int create);
+
+static inline void do_fixup_by_path_string(void *fdt, const char *path,
+					   const char *prop, const char *status)
+{
+	do_fixup_by_path(fdt, path, prop, status, strlen(status) + 1, 1);
+}
+
 void do_fixup_by_prop(void *fdt,
 		      const char *pname, const void *pval, int plen,
 		      const char *prop, const void *val, int len,

Signature · 1 line

1.5.6.5

Jerry Van Baren

2011-08-18 00:29 UTC #1 lore raw

PATCH v3 1/2 fdt: Add a do_fixup_by_path_string() function

Hi Chunhe Lan,

On 08/17/2011 02:24 AM, Chunhe Lan wrote:

[snip]

include/fdt_support.h void do_fixup_by_path(void *fdt, const char *path, const char *prop, · line 45

Quoted 7 lines

include/fdt_support.h +7 -0

+
+static inline void do_fixup_by_path_string(void *fdt, const char *path,
+					   const char *prop, const char *status)
+{
+	do_fixup_by_path(fdt, path, prop, status, strlen(status) + 1, 1);
+}
+

After all the good advice from Scott et al., the patch turns into a pretty trivial one-liner. I am questioning the advantage of calling
do_fixup_by_path_string(fdt, path, prop, status);
vs. simply calling
do_fixup_by_path(fdt, path, prop, status, strlen(status) + 1, 1);

The do_fixup_by_path_string() saves two parameters
"strlen(status) + 1, 1"
at the cost of Yet Another Function. Is it worth it?

Thanks,
gvb

Lan Chunhe

2011-08-18 06:32 UTC #2 lore raw

PATCH v3 1/2 fdt: Add a do_fixup_by_path_string() function

Quoted 20 lines · Jerry Van Baren

Hi Chunhe Lan,
On 08/17/2011 02:24 AM, Chunhe Lan wrote:
[snip]
include/fdt_support.h void do_fixup_by_path(void *fdt, const char *path, const char *prop, · line 45
Quoted 7 lines
include/fdt_support.h +7 -0
+
+static inline void do_fixup_by_path_string(void *fdt, const char *path,
+					   const char *prop, const char *status)
+{
+	do_fixup_by_path(fdt, path, prop, status, strlen(status) + 1, 1);
+}
+
After all the good advice from Scott et al., the patch turns into a pretty trivial one-liner. I am questioning the advantage of calling
do_fixup_by_path_string(fdt, path, prop, status);
vs. simply calling
do_fixup_by_path(fdt, path, prop, status, strlen(status) + 1, 1);

Quoted 3 lines

The do_fixup_by_path_string() saves two parameters
"strlen(status) + 1, 1"
at the cost of Yet Another Function. Is it worth it?

Yes, I think that it is worth. The encapsulation of function is used
for that purpose.

Please refer to do_fixup_by_path_u32(), and it only has two lines code.

Thanks.

-Jack Lan

Scott Wood

2011-08-18 15:14 UTC #3 lore raw

PATCH v3 1/2 fdt: Add a do_fixup_by_path_string() function

In reply to Jerry Van Baren

Quoted 20 lines · Jerry Van Baren

Hi Chunhe Lan,
On 08/17/2011 02:24 AM, Chunhe Lan wrote:
[snip]
include/fdt_support.h void do_fixup_by_path(void *fdt, const char *path, const char *prop, · line 45
Quoted 7 lines
include/fdt_support.h +7 -0
+
+static inline void do_fixup_by_path_string(void *fdt, const char *path,
+                       const char *prop, const char *status)
+{
+    do_fixup_by_path(fdt, path, prop, status, strlen(status) + 1, 1);
+}
+
After all the good advice from Scott et al., the patch turns into a pretty trivial one-liner. I am questioning the advantage of calling
do_fixup_by_path_string(fdt, path, prop, status);
vs. simply calling
do_fixup_by_path(fdt, path, prop, status, strlen(status) + 1, 1);

Quoted 3 lines

The do_fixup_by_path_string() saves two parameters
"strlen(status) + 1, 1"
at the cost of Yet Another Function. Is it worth it?

I think it's a nice convenience, with no runtime cost. It avoids any chance of a mismatch between the string passed to do_fixup_by_path and the string passed to strlen.

More functions versus open-coding is not generally a bad thing.

-Scott

Kumar Gala

2011-08-28 18:15 UTC #4 lore raw

PATCH v3 1/2 fdt: Add a do_fixup_by_path_string() function

In reply to Chunhe Lan

Quoted 5 lines · Chunhe Lan

The do_fixup_by_path_string() will set the specified node's property to the value contained in "status". It would just be a wrapper for do_fixup_by_path() that calls strlen on the argument.

Fix where you break the line, should like more like (use upto 75 chars per line):

The do_fixup_by_path_string() will set the specified node's property to the value contained in "status". It would just be a wrapper for do_fixup_by_path() that calls strlen on the argument.

Quoted 25 lines

Signed-off-by Chunhe Lan <[email protected]>

---
include/fdt_support.h | 7 +++++++

1 files changed, 7 insertions(+), 0 deletions(-)

include/fdt_support.h +7 -0

index 863024f..1de4a1d 100644
@@ -36,6 +36,13 @@ void do_fixup_by_path(void *fdt, const char *path, const char *prop,
 		      const void *val, int len, int create);
 void do_fixup_by_path_u32(void *fdt, const char *path, const char *prop,
 			  u32 val, int create);
+
+static inline void do_fixup_by_path_string(void *fdt, const char *path,
+					   const char *prop, const char *status)
+{
+	do_fixup_by_path(fdt, path, prop, status, strlen(status) + 1, 1);
+}
+
 void do_fixup_by_prop(void *fdt,
 		      const char *pname, const void *pval, int plen,
 		      const char *prop, const void *val, int len,

Signature · 1 line

1.5.6.5

Chunhe Lan

2011-08-29 02:54 UTC #5 lore raw

PATCH v3 1/2 fdt: Add a do_fixup_by_path_string() function

Quoted 11 lines · Kumar Gala

Quoted 5 lines · Chunhe Lan
The do_fixup_by_path_string() will set the specified node's property to the value contained in "status". It would just be a wrapper for do_fixup_by_path() that calls strlen on the argument.
Fix where you break the line, should like more like (use upto 75 chars per line):

     OK.

    Thanks.

    -Jack Lan

Quoted 33 lines

The do_fixup_by_path_string() will set the specified node's property
to the
value contained in "status". It would just be a wrapper for do_fixup_by_path() that calls strlen on the argument.
Quoted 26 lines
Signed-off-by Chunhe Lan <[email protected]>
---
include/fdt_support.h | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
include/fdt_support.h +7 -0
index 863024f..1de4a1d 100644
@@ -36,6 +36,13 @@ void do_fixup_by_path(void *fdt, const char *path,  
 const char *prop,
 		      const void *val, int len, int create);
 void do_fixup_by_path_u32(void *fdt, const char *path, const char *prop,
 			  u32 val, int create);
+
+static inline void do_fixup_by_path_string(void *fdt, const char *path,
+					   const char *prop, const char *status)
+{
+	do_fixup_by_path(fdt, path, prop, status, strlen(status) + 1, 1);
+}
+
 void do_fixup_by_prop(void *fdt,
 		      const char *pname, const void *pval, int plen,
const char *prop, const void *val, int len,
--
1.5.6.5