| Message ID | 502a92e9b53960a6b78fabb48d354cbb5bc1750c.1690187572.git.federico.serafini@bugseng.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [XEN,v2] device_tree: address violations of MISRA C:2012 Rules 8.2 and 8.3 | expand |
On Mon, 24 Jul 2023, Federico Serafini wrote: > Give a name to unnamed parameters thus addressing violations of > MISRA C:2012 Rule 8.2 ("Function types shall be in prototype form with > named parameters"). > Keep consistency between parameter names and types used in function > declarations and the ones used in the corresponding function > definitions, thus addressing violations of MISRA C:2012 Rule 8.3 > ("All declarations of an object or function shall use the same names > and type qualifiers"). > > No functional changes. > > Signed-off-by: Federico Serafini <federico.serafini@bugseng.com> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> > --- > Changes in v2: > - improved consistency in parameter renaming. > --- > xen/common/device_tree.c | 24 ++++++++++++------------ > xen/include/xen/device_tree.h | 16 ++++++++-------- > 2 files changed, 20 insertions(+), 20 deletions(-) > > diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c > index 0677193ab3..d52531dc9f 100644 > --- a/xen/common/device_tree.c > +++ b/xen/common/device_tree.c > @@ -85,11 +85,11 @@ struct dt_bus > unsigned int (*get_flags)(const __be32 *addr); > }; > > -void dt_get_range(const __be32 **cell, const struct dt_device_node *np, > +void dt_get_range(const __be32 **cellp, const struct dt_device_node *np, > u64 *address, u64 *size) > { > - *address = dt_next_cell(dt_n_addr_cells(np), cell); > - *size = dt_next_cell(dt_n_size_cells(np), cell); > + *address = dt_next_cell(dt_n_addr_cells(np), cellp); > + *size = dt_next_cell(dt_n_size_cells(np), cellp); > } > > void dt_set_cell(__be32 **cellp, int size, u64 val) > @@ -993,9 +993,9 @@ int dt_device_get_paddr(const struct dt_device_node *dev, unsigned int index, > } > > int dt_for_each_range(const struct dt_device_node *dev, > - int (*cb)(const struct dt_device_node *, > + int (*cb)(const struct dt_device_node *dev, > uint64_t addr, uint64_t length, > - void *), > + void *data), > void *data) > { > const struct dt_device_node *parent = NULL; > @@ -1164,7 +1164,7 @@ unsigned int dt_number_of_irq(const struct dt_device_node *device) > return (intlen / intsize); > } > > -unsigned int dt_number_of_address(const struct dt_device_node *dev) > +unsigned int dt_number_of_address(const struct dt_device_node *device) > { > const __be32 *prop; > u32 psize; > @@ -1173,20 +1173,20 @@ unsigned int dt_number_of_address(const struct dt_device_node *dev) > int onesize, na, ns; > > /* Get parent & match bus type */ > - parent = dt_get_parent(dev); > + parent = dt_get_parent(device); > if ( parent == NULL ) > return 0; > > bus = dt_match_bus(parent); > if ( !bus ) > return 0; > - bus->count_cells(dev, &na, &ns); > + bus->count_cells(device, &na, &ns); > > if ( !DT_CHECK_COUNTS(na, ns) ) > return 0; > > /* Get "reg" or "assigned-addresses" property */ > - prop = dt_get_property(dev, bus->addresses, &psize); > + prop = dt_get_property(device, bus->addresses, &psize); > if ( prop == NULL ) > return 0; > > @@ -1197,9 +1197,9 @@ unsigned int dt_number_of_address(const struct dt_device_node *dev) > } > > int dt_for_each_irq_map(const struct dt_device_node *dev, > - int (*cb)(const struct dt_device_node *, > - const struct dt_irq *, > - void *), > + int (*cb)(const struct dt_device_node *dev, > + const struct dt_irq *dt_irq, > + void *data), > void *data) > { > const struct dt_device_node *ipar, *tnode, *old = NULL; > diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h > index c2eada7489..8d88159f76 100644 > --- a/xen/include/xen/device_tree.h > +++ b/xen/include/xen/device_tree.h > @@ -538,7 +538,7 @@ bool_t dt_machine_is_compatible(const char *compat); > * Returns a node pointer with refcount incremented, use > * of_node_put() on it when done. > */ > -struct dt_device_node *dt_find_node_by_name(struct dt_device_node *node, > +struct dt_device_node *dt_find_node_by_name(struct dt_device_node *from, > const char *name); > > /** > @@ -639,7 +639,7 @@ unsigned int dt_number_of_address(const struct dt_device_node *device); > * device-tree node. It's the high level pendant to dt_device_get_raw_irq(). > */ > int dt_device_get_irq(const struct dt_device_node *device, unsigned int index, > - struct dt_irq *irq); > + struct dt_irq *out_irq); > > /** > * dt_device_get_raw_irq - Resolve an interrupt for a device without translation > @@ -652,7 +652,7 @@ int dt_device_get_irq(const struct dt_device_node *device, unsigned int index, > */ > int dt_device_get_raw_irq(const struct dt_device_node *device, > unsigned int index, > - struct dt_raw_irq *irq); > + struct dt_raw_irq *out_irq); > > /** > * dt_irq_translate - Translate an irq > @@ -668,9 +668,9 @@ int dt_irq_translate(const struct dt_raw_irq *raw, struct dt_irq *out_irq); > * @data: Caller data passed to callback > */ > int dt_for_each_irq_map(const struct dt_device_node *dev, > - int (*cb)(const struct dt_device_node *, > - const struct dt_irq *, > - void *), > + int (*cb)(const struct dt_device_node *dev, > + const struct dt_irq *dt_irq, > + void *data), > void *data); > > /** > @@ -680,9 +680,9 @@ int dt_for_each_irq_map(const struct dt_device_node *dev, > * @data: Caller data passed to callback > */ > int dt_for_each_range(const struct dt_device_node *dev, > - int (*cb)(const struct dt_device_node *, > + int (*cb)(const struct dt_device_node *dev, > uint64_t addr, uint64_t length, > - void *), > + void *data), > void *data); > > /** > -- > 2.34.1 >
Hi Federico, > -----Original Message----- > From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of > Federico Serafini > Sent: Monday, July 24, 2023 4:40 PM > To: xen-devel@lists.xenproject.org > Cc: consulting@bugseng.com; Federico Serafini > <federico.serafini@bugseng.com>; Stefano Stabellini > <sstabellini@kernel.org>; Julien Grall <julien@xen.org> > Subject: [XEN PATCH v2] device_tree: address violations of MISRA C:2012 > Rules 8.2 and 8.3 > > Give a name to unnamed parameters thus addressing violations of > MISRA C:2012 Rule 8.2 ("Function types shall be in prototype form with > named parameters"). > Keep consistency between parameter names and types used in function > declarations and the ones used in the corresponding function > definitions, thus addressing violations of MISRA C:2012 Rule 8.3 > ("All declarations of an object or function shall use the same names > and type qualifiers"). > > No functional changes. > > Signed-off-by: Federico Serafini <federico.serafini@bugseng.com> Reviewed-by: Henry Wang <Henry.Wang@arm.com> Kind regards, Henry
Hi Federico, On 24/07/2023 09:40, Federico Serafini wrote: > Give a name to unnamed parameters thus addressing violations of > MISRA C:2012 Rule 8.2 ("Function types shall be in prototype form with > named parameters"). > Keep consistency between parameter names and types used in function > declarations and the ones used in the corresponding function > definitions, thus addressing violations of MISRA C:2012 Rule 8.3 > ("All declarations of an object or function shall use the same names > and type qualifiers"). > > No functional changes. > > Signed-off-by: Federico Serafini <federico.serafini@bugseng.com> > --- > Changes in v2: > - improved consistency in parameter renaming. > --- > xen/common/device_tree.c | 24 ++++++++++++------------ > xen/include/xen/device_tree.h | 16 ++++++++-------- > 2 files changed, 20 insertions(+), 20 deletions(-) > > diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c > index 0677193ab3..d52531dc9f 100644 > --- a/xen/common/device_tree.c > +++ b/xen/common/device_tree.c > @@ -85,11 +85,11 @@ struct dt_bus > unsigned int (*get_flags)(const __be32 *addr); > }; > > -void dt_get_range(const __be32 **cell, const struct dt_device_node *np, > +void dt_get_range(const __be32 **cellp, const struct dt_device_node *np, > u64 *address, u64 *size) > { > - *address = dt_next_cell(dt_n_addr_cells(np), cell); > - *size = dt_next_cell(dt_n_size_cells(np), cell); > + *address = dt_next_cell(dt_n_addr_cells(np), cellp); > + *size = dt_next_cell(dt_n_size_cells(np), cellp); > } > > void dt_set_cell(__be32 **cellp, int size, u64 val) > @@ -993,9 +993,9 @@ int dt_device_get_paddr(const struct dt_device_node *dev, unsigned int index, > } > > int dt_for_each_range(const struct dt_device_node *dev, > - int (*cb)(const struct dt_device_node *, > + int (*cb)(const struct dt_device_node *dev, > uint64_t addr, uint64_t length, > - void *), > + void *data), > void *data) > { > const struct dt_device_node *parent = NULL; > @@ -1164,7 +1164,7 @@ unsigned int dt_number_of_irq(const struct dt_device_node *device) > return (intlen / intsize); > } > > -unsigned int dt_number_of_address(const struct dt_device_node *dev) > +unsigned int dt_number_of_address(const struct dt_device_node *device) We have a structure called 'device', so wouldn't this result to violate another MISRA rule because identifiers are re-used? In any case, I would prefer if we keep the shorter version (i.e. 'dev') as this is more common within device_tree.c. We can replace the other 'device' at a leisure pace. Cheers,
Hello Julien, On 25/07/23 12:02, Julien Grall wrote: >> -unsigned int dt_number_of_address(const struct dt_device_node *dev) >> +unsigned int dt_number_of_address(const struct dt_device_node *device) > We have a structure called 'device', so wouldn't this result to violate > another MISRA rule because identifiers are re-used? > > In any case, I would prefer if we keep the shorter version (i.e. 'dev') > as this is more common within device_tree.c. We can replace the other > 'device' at a leisure pace. If you refer to the rule 5.3 ("An identifier declared in an inner scope shall not hide an identifier declared in an outer scope") then no, it is not a violation because there is no hiding. To my knowledge, this does not cause violations of any other MISRA rule. However, I agree with you, the parameter name 'device' is not the best choice. I will propose a v2. Regards
diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c index 0677193ab3..d52531dc9f 100644 --- a/xen/common/device_tree.c +++ b/xen/common/device_tree.c @@ -85,11 +85,11 @@ struct dt_bus unsigned int (*get_flags)(const __be32 *addr); }; -void dt_get_range(const __be32 **cell, const struct dt_device_node *np, +void dt_get_range(const __be32 **cellp, const struct dt_device_node *np, u64 *address, u64 *size) { - *address = dt_next_cell(dt_n_addr_cells(np), cell); - *size = dt_next_cell(dt_n_size_cells(np), cell); + *address = dt_next_cell(dt_n_addr_cells(np), cellp); + *size = dt_next_cell(dt_n_size_cells(np), cellp); } void dt_set_cell(__be32 **cellp, int size, u64 val) @@ -993,9 +993,9 @@ int dt_device_get_paddr(const struct dt_device_node *dev, unsigned int index, } int dt_for_each_range(const struct dt_device_node *dev, - int (*cb)(const struct dt_device_node *, + int (*cb)(const struct dt_device_node *dev, uint64_t addr, uint64_t length, - void *), + void *data), void *data) { const struct dt_device_node *parent = NULL; @@ -1164,7 +1164,7 @@ unsigned int dt_number_of_irq(const struct dt_device_node *device) return (intlen / intsize); } -unsigned int dt_number_of_address(const struct dt_device_node *dev) +unsigned int dt_number_of_address(const struct dt_device_node *device) { const __be32 *prop; u32 psize; @@ -1173,20 +1173,20 @@ unsigned int dt_number_of_address(const struct dt_device_node *dev) int onesize, na, ns; /* Get parent & match bus type */ - parent = dt_get_parent(dev); + parent = dt_get_parent(device); if ( parent == NULL ) return 0; bus = dt_match_bus(parent); if ( !bus ) return 0; - bus->count_cells(dev, &na, &ns); + bus->count_cells(device, &na, &ns); if ( !DT_CHECK_COUNTS(na, ns) ) return 0; /* Get "reg" or "assigned-addresses" property */ - prop = dt_get_property(dev, bus->addresses, &psize); + prop = dt_get_property(device, bus->addresses, &psize); if ( prop == NULL ) return 0; @@ -1197,9 +1197,9 @@ unsigned int dt_number_of_address(const struct dt_device_node *dev) } int dt_for_each_irq_map(const struct dt_device_node *dev, - int (*cb)(const struct dt_device_node *, - const struct dt_irq *, - void *), + int (*cb)(const struct dt_device_node *dev, + const struct dt_irq *dt_irq, + void *data), void *data) { const struct dt_device_node *ipar, *tnode, *old = NULL; diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h index c2eada7489..8d88159f76 100644 --- a/xen/include/xen/device_tree.h +++ b/xen/include/xen/device_tree.h @@ -538,7 +538,7 @@ bool_t dt_machine_is_compatible(const char *compat); * Returns a node pointer with refcount incremented, use * of_node_put() on it when done. */ -struct dt_device_node *dt_find_node_by_name(struct dt_device_node *node, +struct dt_device_node *dt_find_node_by_name(struct dt_device_node *from, const char *name); /** @@ -639,7 +639,7 @@ unsigned int dt_number_of_address(const struct dt_device_node *device); * device-tree node. It's the high level pendant to dt_device_get_raw_irq(). */ int dt_device_get_irq(const struct dt_device_node *device, unsigned int index, - struct dt_irq *irq); + struct dt_irq *out_irq); /** * dt_device_get_raw_irq - Resolve an interrupt for a device without translation @@ -652,7 +652,7 @@ int dt_device_get_irq(const struct dt_device_node *device, unsigned int index, */ int dt_device_get_raw_irq(const struct dt_device_node *device, unsigned int index, - struct dt_raw_irq *irq); + struct dt_raw_irq *out_irq); /** * dt_irq_translate - Translate an irq @@ -668,9 +668,9 @@ int dt_irq_translate(const struct dt_raw_irq *raw, struct dt_irq *out_irq); * @data: Caller data passed to callback */ int dt_for_each_irq_map(const struct dt_device_node *dev, - int (*cb)(const struct dt_device_node *, - const struct dt_irq *, - void *), + int (*cb)(const struct dt_device_node *dev, + const struct dt_irq *dt_irq, + void *data), void *data); /** @@ -680,9 +680,9 @@ int dt_for_each_irq_map(const struct dt_device_node *dev, * @data: Caller data passed to callback */ int dt_for_each_range(const struct dt_device_node *dev, - int (*cb)(const struct dt_device_node *, + int (*cb)(const struct dt_device_node *dev, uint64_t addr, uint64_t length, - void *), + void *data), void *data); /**
Give a name to unnamed parameters thus addressing violations of MISRA C:2012 Rule 8.2 ("Function types shall be in prototype form with named parameters"). Keep consistency between parameter names and types used in function declarations and the ones used in the corresponding function definitions, thus addressing violations of MISRA C:2012 Rule 8.3 ("All declarations of an object or function shall use the same names and type qualifiers"). No functional changes. Signed-off-by: Federico Serafini <federico.serafini@bugseng.com> --- Changes in v2: - improved consistency in parameter renaming. --- xen/common/device_tree.c | 24 ++++++++++++------------ xen/include/xen/device_tree.h | 16 ++++++++-------- 2 files changed, 20 insertions(+), 20 deletions(-)