| Message ID | 20220727223905.624285-2-qemu@ben.fluff.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3,1/5] device_tree: add qemu_fdt_setprop_strings() helper | expand |
On Wed, Jul 27, 2022 at 11:39:01PM +0100, Ben Dooks wrote: > Add a helper to set a property from a set of strings > to reduce the following code: > > static const char * const clint_compat[2] = { > "sifive,clint0", "riscv,clint0" > }; > > qemu_fdt_setprop_string_array(fdt, nodename, "compatible", > (char **)&clint_compat, ARRAY_SIZE(clint_compat)); > > Signed-off-by: Ben Dooks <qemu@ben.fluff.org> > --- > v3; > - fix return value for the call > - add better help text > v2: > - fix node/path in comment > --- > include/sysemu/device_tree.h | 19 +++++++++++++++++++ > 1 file changed, 19 insertions(+) > > diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h > index ef060a9759..83bdfe390e 100644 > --- a/include/sysemu/device_tree.h > +++ b/include/sysemu/device_tree.h > @@ -87,6 +87,25 @@ int qemu_fdt_setprop_string(void *fdt, const char *node_path, > int qemu_fdt_setprop_string_array(void *fdt, const char *node_path, > const char *prop, char **array, int len); > > +/** > + * qemu_fdt_setprop_strings: set a property from a set of strings > + * > + * @fdt: pointer to the dt blob > + * @path: node name > + * @prop: property array > + * > + * This is a helper for the qemu_fdt_setprop_string_array() function > + * which takes a va-arg set of strings instead of having to setup a > + * single use string array. > + */ > +#define qemu_fdt_setprop_strings(fdt, path, prop, ...) \ > + ({ int __ret; do { \ > + static const char * const __strs[] = { __VA_ARGS__ }; \ > + __ret = qemu_fdt_setprop_string_array(fdt, path, prop, \ > + (char **)&__strs, ARRAY_SIZE(__strs)); \ > + } while(0); __ret; }) The do { } while (0) shouldn't be necessary inside the ({}), but I think we should change the places that are checking the return value of qemu_fdt_setprop_string_array() to not check the return value, because it will always be zero. qemu_fdt_setprop_string_array() calls qemu_fdt_setprop() which exits QEMU on error. Then, after there are no callers checking the return value we can change this macro to #define qemu_fdt_setprop_strings(fdt, path, prop, ...) \ do { \ static const char * const __strs[] = { __VA_ARGS__ }; \ qemu_fdt_setprop_string_array(fdt, path, prop, \ (char **)&__strs, ARRAY_SIZE(__strs)); \ } while(0) Thanks, drew
On Thu, Jul 28, 2022 at 11:22:27AM +0200, Andrew Jones wrote: > On Wed, Jul 27, 2022 at 11:39:01PM +0100, Ben Dooks wrote: > > Add a helper to set a property from a set of strings > > to reduce the following code: > > > > static const char * const clint_compat[2] = { > > "sifive,clint0", "riscv,clint0" > > }; > > > > qemu_fdt_setprop_string_array(fdt, nodename, "compatible", > > (char **)&clint_compat, ARRAY_SIZE(clint_compat)); > > > > Signed-off-by: Ben Dooks <qemu@ben.fluff.org> > > --- > > v3; > > - fix return value for the call > > - add better help text > > v2: > > - fix node/path in comment > > --- > > include/sysemu/device_tree.h | 19 +++++++++++++++++++ > > 1 file changed, 19 insertions(+) > > > > diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h > > index ef060a9759..83bdfe390e 100644 > > --- a/include/sysemu/device_tree.h > > +++ b/include/sysemu/device_tree.h > > @@ -87,6 +87,25 @@ int qemu_fdt_setprop_string(void *fdt, const char *node_path, > > int qemu_fdt_setprop_string_array(void *fdt, const char *node_path, > > const char *prop, char **array, int len); > > > > +/** > > + * qemu_fdt_setprop_strings: set a property from a set of strings > > + * > > + * @fdt: pointer to the dt blob > > + * @path: node name > > + * @prop: property array > > + * > > + * This is a helper for the qemu_fdt_setprop_string_array() function > > + * which takes a va-arg set of strings instead of having to setup a > > + * single use string array. > > + */ > > +#define qemu_fdt_setprop_strings(fdt, path, prop, ...) \ > > + ({ int __ret; do { \ > > + static const char * const __strs[] = { __VA_ARGS__ }; \ > > + __ret = qemu_fdt_setprop_string_array(fdt, path, prop, \ > > + (char **)&__strs, ARRAY_SIZE(__strs)); \ > > + } while(0); __ret; }) > > The do { } while (0) shouldn't be necessary inside the ({}), but I > think we should change the places that are checking the return value > of qemu_fdt_setprop_string_array() to not check the return value, > because it will always be zero. qemu_fdt_setprop_string_array() calls > qemu_fdt_setprop() which exits QEMU on error. Then, after there are > no callers checking the return value we can change this macro to I think I did this due to the hw/ppc changes that /do/ check the return code but are different enough to not be trivially changable. I'll go back and make this the original do {} while() tongith and post a v4 for people to look. The hw/ppc stuff can stay as is for now. Thank you for the review.
On Thu, Jul 28, 2022 at 11:22:27AM +0200, Andrew Jones wrote: > On Wed, Jul 27, 2022 at 11:39:01PM +0100, Ben Dooks wrote: > > Add a helper to set a property from a set of strings > > to reduce the following code: > > > > static const char * const clint_compat[2] = { > > "sifive,clint0", "riscv,clint0" > > }; > > > > qemu_fdt_setprop_string_array(fdt, nodename, "compatible", > > (char **)&clint_compat, ARRAY_SIZE(clint_compat)); > > > > Signed-off-by: Ben Dooks <qemu@ben.fluff.org> > > --- > > v3; > > - fix return value for the call > > - add better help text > > v2: > > - fix node/path in comment > > --- > > include/sysemu/device_tree.h | 19 +++++++++++++++++++ > > 1 file changed, 19 insertions(+) > > > > diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h > > index ef060a9759..83bdfe390e 100644 > > --- a/include/sysemu/device_tree.h > > +++ b/include/sysemu/device_tree.h > > @@ -87,6 +87,25 @@ int qemu_fdt_setprop_string(void *fdt, const char *node_path, > > int qemu_fdt_setprop_string_array(void *fdt, const char *node_path, > > const char *prop, char **array, int len); > > > > +/** > > + * qemu_fdt_setprop_strings: set a property from a set of strings > > + * > > + * @fdt: pointer to the dt blob > > + * @path: node name > > + * @prop: property array > > + * > > + * This is a helper for the qemu_fdt_setprop_string_array() function > > + * which takes a va-arg set of strings instead of having to setup a > > + * single use string array. > > + */ > > +#define qemu_fdt_setprop_strings(fdt, path, prop, ...) \ > > + ({ int __ret; do { \ > > + static const char * const __strs[] = { __VA_ARGS__ }; \ > > + __ret = qemu_fdt_setprop_string_array(fdt, path, prop, \ > > + (char **)&__strs, ARRAY_SIZE(__strs)); \ > > + } while(0); __ret; }) > > The do { } while (0) shouldn't be necessary inside the ({}), but I > think we should change the places that are checking the return value > of qemu_fdt_setprop_string_array() to not check the return value, > because it will always be zero. qemu_fdt_setprop_string_array() calls > qemu_fdt_setprop() which exits QEMU on error. Then, after there are > no callers checking the return value we can change this macro to > > #define qemu_fdt_setprop_strings(fdt, path, prop, ...) \ > do { \ > static const char * const __strs[] = { __VA_ARGS__ }; \ Eh, I just realized I didn't notice the static the first time I read this and so I copy+pasted here in my suggestion. Sorry about that. Thanks, drew > qemu_fdt_setprop_string_array(fdt, path, prop, \ > (char **)&__strs, ARRAY_SIZE(__strs)); \ > } while(0) > > > Thanks, > drew
diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h index ef060a9759..83bdfe390e 100644 --- a/include/sysemu/device_tree.h +++ b/include/sysemu/device_tree.h @@ -87,6 +87,25 @@ int qemu_fdt_setprop_string(void *fdt, const char *node_path, int qemu_fdt_setprop_string_array(void *fdt, const char *node_path, const char *prop, char **array, int len); +/** + * qemu_fdt_setprop_strings: set a property from a set of strings + * + * @fdt: pointer to the dt blob + * @path: node name + * @prop: property array + * + * This is a helper for the qemu_fdt_setprop_string_array() function + * which takes a va-arg set of strings instead of having to setup a + * single use string array. + */ +#define qemu_fdt_setprop_strings(fdt, path, prop, ...) \ + ({ int __ret; do { \ + static const char * const __strs[] = { __VA_ARGS__ }; \ + __ret = qemu_fdt_setprop_string_array(fdt, path, prop, \ + (char **)&__strs, ARRAY_SIZE(__strs)); \ + } while(0); __ret; }) + + int qemu_fdt_setprop_phandle(void *fdt, const char *node_path, const char *property, const char *target_node_path);
Add a helper to set a property from a set of strings to reduce the following code: static const char * const clint_compat[2] = { "sifive,clint0", "riscv,clint0" }; qemu_fdt_setprop_string_array(fdt, nodename, "compatible", (char **)&clint_compat, ARRAY_SIZE(clint_compat)); Signed-off-by: Ben Dooks <qemu@ben.fluff.org> --- v3; - fix return value for the call - add better help text v2: - fix node/path in comment --- include/sysemu/device_tree.h | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)