Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
---
Changes in v3:
- the fixes are based solely to Eclair findings (the tool has been
adjusted to report only those violations that can result to a bug)
- reflect this in the commit title
xen/include/xen/device_tree.h | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h
index a28937d12ae8..7839a199e311 100644
--- a/xen/include/xen/device_tree.h
+++ b/xen/include/xen/device_tree.h
@@ -37,11 +37,11 @@ struct dt_device_match {
const void *data;
};
-#define __DT_MATCH_PATH(p) .path = p
-#define __DT_MATCH_TYPE(typ) .type = typ
-#define __DT_MATCH_COMPATIBLE(compat) .compatible = compat
+#define __DT_MATCH_PATH(p) .path = (p)
+#define __DT_MATCH_TYPE(typ) .type = (typ)
+#define __DT_MATCH_COMPATIBLE(compat) .compatible = (compat)
#define __DT_MATCH_NOT_AVAILABLE() .not_available = 1
-#define __DT_MATCH_PROP(p) .prop = p
+#define __DT_MATCH_PROP(p) .prop = (p)
#define DT_MATCH_PATH(p) { __DT_MATCH_PATH(p) }
#define DT_MATCH_TYPE(typ) { __DT_MATCH_TYPE(typ) }
@@ -226,13 +226,13 @@ dt_find_interrupt_controller(const struct dt_device_match *matches);
#define DT_ROOT_NODE_SIZE_CELLS_DEFAULT 1
#define dt_for_each_property_node(dn, pp) \
- for ( pp = dn->properties; pp != NULL; pp = pp->next )
+ for ( pp = (dn)->properties; (pp) != NULL; pp = (pp)->next )
#define dt_for_each_device_node(dt, dn) \
- for ( dn = dt; dn != NULL; dn = dn->allnext )
+ for ( dn = dt; (dn) != NULL; dn = (dn)->allnext )
#define dt_for_each_child_node(dt, dn) \
- for ( dn = dt->child; dn != NULL; dn = dn->sibling )
+ for ( dn = (dt)->child; (dn) != NULL; dn = (dn)->sibling )
/* Helper to read a big number; size is in cells (not bytes) */
static inline u64 dt_read_number(const __be32 *cell, int size)
--
2.37.2
> On 3 Feb 2023, at 19:09, Xenia Ragiadakou <burzalodowa@gmail.com> wrote:
>
I’m not really a supporter of empty commit message, but it’s up to the maintainer :)
For me the changes looks good
Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
> Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
> ---
>
> Changes in v3:
> - the fixes are based solely to Eclair findings (the tool has been
> adjusted to report only those violations that can result to a bug)
> - reflect this in the commit title
>
> xen/include/xen/device_tree.h | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h
> index a28937d12ae8..7839a199e311 100644
> --- a/xen/include/xen/device_tree.h
> +++ b/xen/include/xen/device_tree.h
> @@ -37,11 +37,11 @@ struct dt_device_match {
> const void *data;
> };
>
> -#define __DT_MATCH_PATH(p) .path = p
> -#define __DT_MATCH_TYPE(typ) .type = typ
> -#define __DT_MATCH_COMPATIBLE(compat) .compatible = compat
> +#define __DT_MATCH_PATH(p) .path = (p)
> +#define __DT_MATCH_TYPE(typ) .type = (typ)
> +#define __DT_MATCH_COMPATIBLE(compat) .compatible = (compat)
> #define __DT_MATCH_NOT_AVAILABLE() .not_available = 1
> -#define __DT_MATCH_PROP(p) .prop = p
> +#define __DT_MATCH_PROP(p) .prop = (p)
>
> #define DT_MATCH_PATH(p) { __DT_MATCH_PATH(p) }
> #define DT_MATCH_TYPE(typ) { __DT_MATCH_TYPE(typ) }
> @@ -226,13 +226,13 @@ dt_find_interrupt_controller(const struct dt_device_match *matches);
> #define DT_ROOT_NODE_SIZE_CELLS_DEFAULT 1
>
> #define dt_for_each_property_node(dn, pp) \
> - for ( pp = dn->properties; pp != NULL; pp = pp->next )
> + for ( pp = (dn)->properties; (pp) != NULL; pp = (pp)->next )
>
> #define dt_for_each_device_node(dt, dn) \
> - for ( dn = dt; dn != NULL; dn = dn->allnext )
> + for ( dn = dt; (dn) != NULL; dn = (dn)->allnext )
>
> #define dt_for_each_child_node(dt, dn) \
> - for ( dn = dt->child; dn != NULL; dn = dn->sibling )
> + for ( dn = (dt)->child; (dn) != NULL; dn = (dn)->sibling )
>
> /* Helper to read a big number; size is in cells (not bytes) */
> static inline u64 dt_read_number(const __be32 *cell, int size)
> --
> 2.37.2
>
>
Hi, On 07/02/2023 10:23, Luca Fancellu wrote: > > >> On 3 Feb 2023, at 19:09, Xenia Ragiadakou <burzalodowa@gmail.com> wrote: >> > > I’m not really a supporter of empty commit message, but it’s up to the maintainer :) +1. In this case a brief summary of the rule would be handy for those that are not well-versed with MISRA. This can be dealt on commit if you propose a new commit message. > > For me the changes looks good > > Reviewed-by: Luca Fancellu <luca.fancellu@arm.com> Cheers, -- Julien Grall
On 2/7/23 12:39, Julien Grall wrote: > Hi, > > On 07/02/2023 10:23, Luca Fancellu wrote: >> >> >>> On 3 Feb 2023, at 19:09, Xenia Ragiadakou <burzalodowa@gmail.com> wrote: >>> >> >> I’m not really a supporter of empty commit message, but it’s up to the >> maintainer :) > > +1. In this case a brief summary of the rule would be handy for those > that are not well-versed with MISRA. > > This can be dealt on commit if you propose a new commit message. I 'm refrained from stating the rule as is because it is strict and it is not applied as is. "Add parentheses around macro parameters when the precedence and associativity of the performed operators can lead to unintended order of evaluation." Is this ok? > >> >> For me the changes looks good >> >> Reviewed-by: Luca Fancellu <luca.fancellu@arm.com> > > Cheers, > -- Xenia
On 07/02/2023 10:46, Xenia Ragiadakou wrote: > > On 2/7/23 12:39, Julien Grall wrote: >> Hi, >> >> On 07/02/2023 10:23, Luca Fancellu wrote: >>> >>> >>>> On 3 Feb 2023, at 19:09, Xenia Ragiadakou <burzalodowa@gmail.com> >>>> wrote: >>>> >>> >>> I’m not really a supporter of empty commit message, but it’s up to >>> the maintainer :) >> >> +1. In this case a brief summary of the rule would be handy for those >> that are not well-versed with MISRA. >> >> This can be dealt on commit if you propose a new commit message. > > I 'm refrained from stating the rule as is because it is strict and it > is not applied as is. I am a bit confused with this statement. From misra/..., we are supporting rule 20.7. So why aren't applying it strictly? And if it is not applied as-is, shouldn't we document the violation (if any)? > > "Add parentheses around macro parameters when the precedence and > associativity of the performed operators can lead to unintended order of > evaluation." > > Is this ok? I am OK with this. Is there any ID from Eclair that could be used to track each error (and so we can confirm they have disappeared)? Cheers, -- Julien Grall
On 2/7/23 14:25, Julien Grall wrote: > > > On 07/02/2023 10:46, Xenia Ragiadakou wrote: >> >> On 2/7/23 12:39, Julien Grall wrote: >>> Hi, >>> >>> On 07/02/2023 10:23, Luca Fancellu wrote: >>>> >>>> >>>>> On 3 Feb 2023, at 19:09, Xenia Ragiadakou <burzalodowa@gmail.com> >>>>> wrote: >>>>> >>>> >>>> I’m not really a supporter of empty commit message, but it’s up to >>>> the maintainer :) >>> >>> +1. In this case a brief summary of the rule would be handy for those >>> that are not well-versed with MISRA. >>> >>> This can be dealt on commit if you propose a new commit message. >> >> I 'm refrained from stating the rule as is because it is strict and it >> is not applied as is. > > I am a bit confused with this statement. From misra/..., we are > supporting rule 20.7. So why aren't applying it strictly? > > And if it is not applied as-is, shouldn't we document the violation (if > any)? I applied it strictly on v2, but there was no review. Then Eclair was adjusted to have a less strict approach. Still there is a finding asking to add parentheses around dt in dt_for_each_device_node(dt, dn), i.e dn = (dt);, to which AFAIK you object. > >> >> "Add parentheses around macro parameters when the precedence and >> associativity of the performed operators can lead to unintended order >> of evaluation." >> >> Is this ok? > > I am OK with this. Is there any ID from Eclair that could be used to > track each error (and so we can confirm they have disappeared)? I am not aware of any. The patch can be decoupled from misra and Eclair (I mean have a generic commit title) and just mention in the commit message that it fixes some Eclair findings for MISRA C rule 20.7. > > Cheers, > -- Xenia
Hi, On 07/02/2023 12:46, Xenia Ragiadakou wrote: > On 2/7/23 14:25, Julien Grall wrote: >> >> >> On 07/02/2023 10:46, Xenia Ragiadakou wrote: >>> >>> On 2/7/23 12:39, Julien Grall wrote: >>>> Hi, >>>> >>>> On 07/02/2023 10:23, Luca Fancellu wrote: >>>>> >>>>> >>>>>> On 3 Feb 2023, at 19:09, Xenia Ragiadakou <burzalodowa@gmail.com> >>>>>> wrote: >>>>>> >>>>> >>>>> I’m not really a supporter of empty commit message, but it’s up to >>>>> the maintainer :) >>>> >>>> +1. In this case a brief summary of the rule would be handy for >>>> those that are not well-versed with MISRA. >>>> >>>> This can be dealt on commit if you propose a new commit message. >>> >>> I 'm refrained from stating the rule as is because it is strict and >>> it is not applied as is. >> >> I am a bit confused with this statement. From misra/..., we are >> supporting rule 20.7. So why aren't applying it strictly? >> >> And if it is not applied as-is, shouldn't we document the violation >> (if any)? > > I applied it strictly on v2, but there was no review. Ah! In general it is best to ping if there are no answers. > Then Eclair was adjusted to have a less strict approach. Still there is > a finding asking to add parentheses around dt in > dt_for_each_device_node(dt, dn), i.e dn = (dt);, to which AFAIK you object. Are you referring to the discussion in [1]? If so, I wasn't totally opposed to the suggestion so long we are consistent. > >> >>> >>> "Add parentheses around macro parameters when the precedence and >>> associativity of the performed operators can lead to unintended order >>> of evaluation." >>> >>> Is this ok? >> >> I am OK with this. Is there any ID from Eclair that could be used to >> track each error (and so we can confirm they have disappeared)? > > I am not aware of any. Hmmm ok. It might be a nice feature to suggest in Eclair because anyone can check whether an issue was resolved. Here, I don't exactly know what to check in Eclair. So I have to rely on you. Anyway, nothing that can be fixed for this commit. > > The patch can be decoupled from misra and Eclair (I mean have a generic > commit title) and just mention in the commit message that it fixes some > Eclair findings for MISRA C rule 20.7. I have a slight preference for a more generic title. But the current one also work for me. I will commit later on. Cheers, [1] b2f2d1e7-0c18-206f-5e9d-d0115e398840@xen.org > >> >> Cheers, >> > -- Julien Grall
Hi Julien, On 2/7/23 15:01, Julien Grall wrote: > Hi, > > On 07/02/2023 12:46, Xenia Ragiadakou wrote: >> On 2/7/23 14:25, Julien Grall wrote: >>> >>> >>> On 07/02/2023 10:46, Xenia Ragiadakou wrote: >>>> >>>> On 2/7/23 12:39, Julien Grall wrote: >>>>> Hi, >>>>> >>>>> On 07/02/2023 10:23, Luca Fancellu wrote: >>>>>> >>>>>> >>>>>>> On 3 Feb 2023, at 19:09, Xenia Ragiadakou <burzalodowa@gmail.com> >>>>>>> wrote: >>>>>>> >>>>>> >>>>>> I’m not really a supporter of empty commit message, but it’s up to >>>>>> the maintainer :) >>>>> >>>>> +1. In this case a brief summary of the rule would be handy for >>>>> those that are not well-versed with MISRA. >>>>> >>>>> This can be dealt on commit if you propose a new commit message. >>>> >>>> I 'm refrained from stating the rule as is because it is strict and >>>> it is not applied as is. >>> >>> I am a bit confused with this statement. From misra/..., we are >>> supporting rule 20.7. So why aren't applying it strictly? >>> >>> And if it is not applied as-is, shouldn't we document the violation >>> (if any)? >> >> I applied it strictly on v2, but there was no review. > > Ah! In general it is best to ping if there are no answers. Me too I ve forgotten about it. A ticket in gitlab reminded me that it was pending. > >> Then Eclair was adjusted to have a less strict approach. Still there >> is a finding asking to add parentheses around dt in >> dt_for_each_device_node(dt, dn), i.e dn = (dt);, to which AFAIK you >> object. > > Are you referring to the discussion in [1]? If so, I wasn't totally > opposed to the suggestion so long we are consistent. I am not able to find [1]. I 'm referring to the discussion in 20221220085100.22848-6-luca.fancellu@arm.com and 20220728134943.1185621-1-burzalodowa@gmail.com > >> >>> >>>> >>>> "Add parentheses around macro parameters when the precedence and >>>> associativity of the performed operators can lead to unintended >>>> order of evaluation." >>>> >>>> Is this ok? >>> >>> I am OK with this. Is there any ID from Eclair that could be used to >>> track each error (and so we can confirm they have disappeared)? >> >> I am not aware of any. > Hmmm ok. It might be a nice feature to suggest in Eclair because anyone > can check whether an issue was resolved. Currently, the violations in Eclair are reported per macro call (I guess because it is acceptable to have parentheses around the macro args) so it is difficult to track all of them. > > Here, I don't exactly know what to check in Eclair. So I have to rely on > you. Anyway, nothing that can be fixed for this commit. > >> >> The patch can be decoupled from misra and Eclair (I mean have a >> generic commit title) and just mention in the commit message that it >> fixes some Eclair findings for MISRA C rule 20.7. > > I have a slight preference for a more generic title. But the current one > also work for me. It can be changed into "xen/device_tree: add parentheses around macro parameters" > > I will commit later on. Do you want me to send a v4? > > Cheers, > > [1] b2f2d1e7-0c18-206f-5e9d-d0115e398840@xen.org >> >>> >>> Cheers, >>> >> > -- Xenia
On 07/02/2023 13:59, Xenia Ragiadakou wrote:
> Hi Julien,
Hi Xenia,
>
> On 2/7/23 15:01, Julien Grall wrote:
>> Hi,
>>
>> On 07/02/2023 12:46, Xenia Ragiadakou wrote:
>>> On 2/7/23 14:25, Julien Grall wrote:
>>>>
>>>>
>>>> On 07/02/2023 10:46, Xenia Ragiadakou wrote:
>>>>>
>>>>> On 2/7/23 12:39, Julien Grall wrote:
>>>>>> Hi,
>>>>>>
>>>>>> On 07/02/2023 10:23, Luca Fancellu wrote:
>>>>>>>
>>>>>>>
>>>>>>>> On 3 Feb 2023, at 19:09, Xenia Ragiadakou
>>>>>>>> <burzalodowa@gmail.com> wrote:
>>>>>>>>
>>>>>>>
>>>>>>> I’m not really a supporter of empty commit message, but it’s up
>>>>>>> to the maintainer :)
>>>>>>
>>>>>> +1. In this case a brief summary of the rule would be handy for
>>>>>> those that are not well-versed with MISRA.
>>>>>>
>>>>>> This can be dealt on commit if you propose a new commit message.
>>>>>
>>>>> I 'm refrained from stating the rule as is because it is strict and
>>>>> it is not applied as is.
>>>>
>>>> I am a bit confused with this statement. From misra/..., we are
>>>> supporting rule 20.7. So why aren't applying it strictly?
>>>>
>>>> And if it is not applied as-is, shouldn't we document the violation
>>>> (if any)?
>>>
>>> I applied it strictly on v2, but there was no review.
>>
>> Ah! In general it is best to ping if there are no answers.
>
> Me too I ve forgotten about it. A ticket in gitlab reminded me that it
> was pending.
>
>>
>>> Then Eclair was adjusted to have a less strict approach. Still there
>>> is a finding asking to add parentheses around dt in
>>> dt_for_each_device_node(dt, dn), i.e dn = (dt);, to which AFAIK you
>>> object.
>>
>> Are you referring to the discussion in [1]? If so, I wasn't totally
>> opposed to the suggestion so long we are consistent.
>
> I am not able to find [1].
https://lore.kernel.org/xen-devel/b2f2d1e7-0c18-206f-5e9d-d0115e398840@xen.org/
which is...
> I 'm referring to the discussion in
> 20221220085100.22848-6-luca.fancellu@arm.com and
> 20220728134943.1185621-1-burzalodowa@gmail.com
... a reply to this message. At the end of that reply, I said that I
wasn't totally against adding the parentheses but we should be
consistent in how we do it.
>
>>
>>>
>>>>
>>>>>
>>>>> "Add parentheses around macro parameters when the precedence and
>>>>> associativity of the performed operators can lead to unintended
>>>>> order of evaluation."
>>>>>
>>>>> Is this ok?
>>>>
>>>> I am OK with this. Is there any ID from Eclair that could be used to
>>>> track each error (and so we can confirm they have disappeared)?
>>>
>>> I am not aware of any.
>> Hmmm ok. It might be a nice feature to suggest in Eclair because
>> anyone can check whether an issue was resolved.
>
> Currently, the violations in Eclair are reported per macro call (I guess
> because it is acceptable to have parentheses around the macro args) so
> it is difficult to track all of them.
>
>>
>> Here, I don't exactly know what to check in Eclair. So I have to rely
>> on you. Anyway, nothing that can be fixed for this commit.
>>
>>>
>>> The patch can be decoupled from misra and Eclair (I mean have a
>>> generic commit title) and just mention in the commit message that it
>>> fixes some Eclair findings for MISRA C rule 20.7.
>>
>> I have a slight preference for a more generic title. But the current
>> one also work for me.
>
> It can be changed into "xen/device_tree: add parentheses around macro
> parameters"
>
>>
>> I will commit later on.
>
> Do you want me to send a v4?
No need. It is now merged with the following commit message:
xen/device_tree: add parentheses around macro parameters
Add parentheses around macro parameters when the precedence and
associativity of the performed operators can lead to unintended
order of evaluation.
This is fixing some ECLAIR finding for Misra Rule 20.7.
Link:
https://lore.kernel.org/xen-devel/20230203190908.211541-2-burzalodowa@gmail.com/
Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
[jgrall: Reworded the commit message]
Acked-by: Julien Grall <jgrall@amazon.com>
Cheers,
--
Julien Grall
On 2/8/23 10:38, Julien Grall wrote: > > > On 07/02/2023 13:59, Xenia Ragiadakou wrote: >> Hi Julien, > > Hi Xenia, > >> >> On 2/7/23 15:01, Julien Grall wrote: >>> Hi, >>> >>> On 07/02/2023 12:46, Xenia Ragiadakou wrote: >>>> On 2/7/23 14:25, Julien Grall wrote: >>>>> >>>>> >>>>> On 07/02/2023 10:46, Xenia Ragiadakou wrote: >>>>>> >>>>>> On 2/7/23 12:39, Julien Grall wrote: >>>>>>> Hi, >>>>>>> >>>>>>> On 07/02/2023 10:23, Luca Fancellu wrote: >>>>>>>> >>>>>>>> >>>>>>>>> On 3 Feb 2023, at 19:09, Xenia Ragiadakou >>>>>>>>> <burzalodowa@gmail.com> wrote: >>>>>>>>> >>>>>>>> >>>>>>>> I’m not really a supporter of empty commit message, but it’s up >>>>>>>> to the maintainer :) >>>>>>> >>>>>>> +1. In this case a brief summary of the rule would be handy for >>>>>>> those that are not well-versed with MISRA. >>>>>>> >>>>>>> This can be dealt on commit if you propose a new commit message. >>>>>> >>>>>> I 'm refrained from stating the rule as is because it is strict >>>>>> and it is not applied as is. >>>>> >>>>> I am a bit confused with this statement. From misra/..., we are >>>>> supporting rule 20.7. So why aren't applying it strictly? >>>>> >>>>> And if it is not applied as-is, shouldn't we document the violation >>>>> (if any)? >>>> >>>> I applied it strictly on v2, but there was no review. >>> >>> Ah! In general it is best to ping if there are no answers. >> >> Me too I ve forgotten about it. A ticket in gitlab reminded me that it >> was pending. >> >>> >>>> Then Eclair was adjusted to have a less strict approach. Still there >>>> is a finding asking to add parentheses around dt in >>>> dt_for_each_device_node(dt, dn), i.e dn = (dt);, to which AFAIK you >>>> object. >>> >>> Are you referring to the discussion in [1]? If so, I wasn't totally >>> opposed to the suggestion so long we are consistent. >> >> I am not able to find [1]. > > https://lore.kernel.org/xen-devel/b2f2d1e7-0c18-206f-5e9d-d0115e398840@xen.org/ > > which is... > > >> I 'm referring to the discussion in >> 20221220085100.22848-6-luca.fancellu@arm.com and >> 20220728134943.1185621-1-burzalodowa@gmail.com > > ... a reply to this message. At the end of that reply, I said that I > wasn't totally against adding the parentheses but we should be > consistent in how we do it. Ok, I must have got it wrong. > >> >>> >>>> >>>>> >>>>>> >>>>>> "Add parentheses around macro parameters when the precedence and >>>>>> associativity of the performed operators can lead to unintended >>>>>> order of evaluation." >>>>>> >>>>>> Is this ok? >>>>> >>>>> I am OK with this. Is there any ID from Eclair that could be used >>>>> to track each error (and so we can confirm they have disappeared)? >>>> >>>> I am not aware of any. >>> Hmmm ok. It might be a nice feature to suggest in Eclair because >>> anyone can check whether an issue was resolved. >> >> Currently, the violations in Eclair are reported per macro call (I >> guess because it is acceptable to have parentheses around the macro >> args) so it is difficult to track all of them. >> >>> >>> Here, I don't exactly know what to check in Eclair. So I have to rely >>> on you. Anyway, nothing that can be fixed for this commit. >>> >>>> >>>> The patch can be decoupled from misra and Eclair (I mean have a >>>> generic commit title) and just mention in the commit message that it >>>> fixes some Eclair findings for MISRA C rule 20.7. >>> >>> I have a slight preference for a more generic title. But the current >>> one also work for me. >> >> It can be changed into "xen/device_tree: add parentheses around macro >> parameters" >> >>> >>> I will commit later on. >> >> Do you want me to send a v4? > > No need. It is now merged with the following commit message: > > xen/device_tree: add parentheses around macro parameters > > Add parentheses around macro parameters when the precedence and > associativity of the performed operators can lead to unintended > order of evaluation. > > This is fixing some ECLAIR finding for Misra Rule 20.7. > > Link: > https://lore.kernel.org/xen-devel/20230203190908.211541-2-burzalodowa@gmail.com/ > Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com> > Reviewed-by: Luca Fancellu <luca.fancellu@arm.com> > [jgrall: Reworded the commit message] > Acked-by: Julien Grall <jgrall@amazon.com> Thanks a lot. > > Cheers, > -- Xenia
Hi Xenia,
> On 3 Feb 2023, at 19:09, Xenia Ragiadakou <burzalodowa@gmail.com> wrote:
>
> Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
> ---
>
> Changes in v3:
> - the fixes are based solely to Eclair findings (the tool has been
> adjusted to report only those violations that can result to a bug)
> - reflect this in the commit title
>
> xen/include/xen/device_tree.h | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h
> index a28937d12ae8..7839a199e311 100644
> --- a/xen/include/xen/device_tree.h
> +++ b/xen/include/xen/device_tree.h
> @@ -37,11 +37,11 @@ struct dt_device_match {
> const void *data;
> };
>
> -#define __DT_MATCH_PATH(p) .path = p
> -#define __DT_MATCH_TYPE(typ) .type = typ
> -#define __DT_MATCH_COMPATIBLE(compat) .compatible = compat
> +#define __DT_MATCH_PATH(p) .path = (p)
> +#define __DT_MATCH_TYPE(typ) .type = (typ)
> +#define __DT_MATCH_COMPATIBLE(compat) .compatible = (compat)
> #define __DT_MATCH_NOT_AVAILABLE() .not_available = 1
> -#define __DT_MATCH_PROP(p) .prop = p
> +#define __DT_MATCH_PROP(p) .prop = (p)
>
> #define DT_MATCH_PATH(p) { __DT_MATCH_PATH(p) }
> #define DT_MATCH_TYPE(typ) { __DT_MATCH_TYPE(typ) }
> @@ -226,13 +226,13 @@ dt_find_interrupt_controller(const struct dt_device_match *matches);
> #define DT_ROOT_NODE_SIZE_CELLS_DEFAULT 1
>
> #define dt_for_each_property_node(dn, pp) \
> - for ( pp = dn->properties; pp != NULL; pp = pp->next )
> + for ( pp = (dn)->properties; (pp) != NULL; pp = (pp)->next )
>
> #define dt_for_each_device_node(dt, dn) \
> - for ( dn = dt; dn != NULL; dn = dn->allnext )
> + for ( dn = dt; (dn) != NULL; dn = (dn)->allnext )
>
> #define dt_for_each_child_node(dt, dn) \
> - for ( dn = dt->child; dn != NULL; dn = dn->sibling )
> + for ( dn = (dt)->child; (dn) != NULL; dn = (dn)->sibling )
>
> /* Helper to read a big number; size is in cells (not bytes) */
> static inline u64 dt_read_number(const __be32 *cell, int size)
> --
> 2.37.2
>
>
While the changes looks sensible to me, I’ve had a look in eclair but I was unable to find the findings,
here what findings I have in the latest report:
https://eclairit.com:8443/job/XEN/Target=ARM64,agent=docker1/lastBuild/eclair/packageName.832204620/fileName.1811675806/
Hi Luca,
On 06/02/2023 15:31, Luca Fancellu wrote:
>
>
> Hi Xenia,
>
>> On 3 Feb 2023, at 19:09, Xenia Ragiadakou <burzalodowa@gmail.com> wrote:
>>
>> Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
>> ---
>>
>> Changes in v3:
>> - the fixes are based solely to Eclair findings (the tool has been
>> adjusted to report only those violations that can result to a bug)
>> - reflect this in the commit title
>>
>> xen/include/xen/device_tree.h | 14 +++++++-------
>> 1 file changed, 7 insertions(+), 7 deletions(-)
>>
>> diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h
>> index a28937d12ae8..7839a199e311 100644
>> --- a/xen/include/xen/device_tree.h
>> +++ b/xen/include/xen/device_tree.h
>> @@ -37,11 +37,11 @@ struct dt_device_match {
>> const void *data;
>> };
>>
>> -#define __DT_MATCH_PATH(p) .path = p
>> -#define __DT_MATCH_TYPE(typ) .type = typ
>> -#define __DT_MATCH_COMPATIBLE(compat) .compatible = compat
>> +#define __DT_MATCH_PATH(p) .path = (p)
>> +#define __DT_MATCH_TYPE(typ) .type = (typ)
>> +#define __DT_MATCH_COMPATIBLE(compat) .compatible = (compat)
>> #define __DT_MATCH_NOT_AVAILABLE() .not_available = 1
>> -#define __DT_MATCH_PROP(p) .prop = p
>> +#define __DT_MATCH_PROP(p) .prop = (p)
>>
>> #define DT_MATCH_PATH(p) { __DT_MATCH_PATH(p) }
>> #define DT_MATCH_TYPE(typ) { __DT_MATCH_TYPE(typ) }
>> @@ -226,13 +226,13 @@ dt_find_interrupt_controller(const struct dt_device_match *matches);
>> #define DT_ROOT_NODE_SIZE_CELLS_DEFAULT 1
>>
>> #define dt_for_each_property_node(dn, pp) \
>> - for ( pp = dn->properties; pp != NULL; pp = pp->next )
>> + for ( pp = (dn)->properties; (pp) != NULL; pp = (pp)->next )
>>
>> #define dt_for_each_device_node(dt, dn) \
>> - for ( dn = dt; dn != NULL; dn = dn->allnext )
>> + for ( dn = dt; (dn) != NULL; dn = (dn)->allnext )
>>
>> #define dt_for_each_child_node(dt, dn) \
>> - for ( dn = dt->child; dn != NULL; dn = dn->sibling )
>> + for ( dn = (dt)->child; (dn) != NULL; dn = (dn)->sibling )
>>
>> /* Helper to read a big number; size is in cells (not bytes) */
>> static inline u64 dt_read_number(const __be32 *cell, int size)
>> --
>> 2.37.2
>>
>>
>
> While the changes looks sensible to me, I’ve had a look in eclair but I was unable to find the findings,
> here what findings I have in the latest report:
> https://eclairit.com:8443/job/XEN/Target=ARM64,agent=docker1/lastBuild/eclair/packageName.832204620/fileName.1811675806/
Eclair does not report violations at the definition but rather at the use.
Check domain_build.c for example to see the reports for 20.7 related to these macros.
~Michal
> On 6 Feb 2023, at 14:37, Michal Orzel <michal.orzel@amd.com> wrote:
>
> Hi Luca,
>
> On 06/02/2023 15:31, Luca Fancellu wrote:
>>
>>
>> Hi Xenia,
>>
>>> On 3 Feb 2023, at 19:09, Xenia Ragiadakou <burzalodowa@gmail.com> wrote:
>>>
>>> Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
>>> ---
>>>
>>> Changes in v3:
>>> - the fixes are based solely to Eclair findings (the tool has been
>>> adjusted to report only those violations that can result to a bug)
>>> - reflect this in the commit title
>>>
>>> xen/include/xen/device_tree.h | 14 +++++++-------
>>> 1 file changed, 7 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h
>>> index a28937d12ae8..7839a199e311 100644
>>> --- a/xen/include/xen/device_tree.h
>>> +++ b/xen/include/xen/device_tree.h
>>> @@ -37,11 +37,11 @@ struct dt_device_match {
>>> const void *data;
>>> };
>>>
>>> -#define __DT_MATCH_PATH(p) .path = p
>>> -#define __DT_MATCH_TYPE(typ) .type = typ
>>> -#define __DT_MATCH_COMPATIBLE(compat) .compatible = compat
>>> +#define __DT_MATCH_PATH(p) .path = (p)
>>> +#define __DT_MATCH_TYPE(typ) .type = (typ)
>>> +#define __DT_MATCH_COMPATIBLE(compat) .compatible = (compat)
>>> #define __DT_MATCH_NOT_AVAILABLE() .not_available = 1
>>> -#define __DT_MATCH_PROP(p) .prop = p
>>> +#define __DT_MATCH_PROP(p) .prop = (p)
>>>
>>> #define DT_MATCH_PATH(p) { __DT_MATCH_PATH(p) }
>>> #define DT_MATCH_TYPE(typ) { __DT_MATCH_TYPE(typ) }
>>> @@ -226,13 +226,13 @@ dt_find_interrupt_controller(const struct dt_device_match *matches);
>>> #define DT_ROOT_NODE_SIZE_CELLS_DEFAULT 1
>>>
>>> #define dt_for_each_property_node(dn, pp) \
>>> - for ( pp = dn->properties; pp != NULL; pp = pp->next )
>>> + for ( pp = (dn)->properties; (pp) != NULL; pp = (pp)->next )
>>>
>>> #define dt_for_each_device_node(dt, dn) \
>>> - for ( dn = dt; dn != NULL; dn = dn->allnext )
>>> + for ( dn = dt; (dn) != NULL; dn = (dn)->allnext )
>>>
>>> #define dt_for_each_child_node(dt, dn) \
>>> - for ( dn = dt->child; dn != NULL; dn = dn->sibling )
>>> + for ( dn = (dt)->child; (dn) != NULL; dn = (dn)->sibling )
>>>
>>> /* Helper to read a big number; size is in cells (not bytes) */
>>> static inline u64 dt_read_number(const __be32 *cell, int size)
>>> --
>>> 2.37.2
>>>
>>>
>>
>> While the changes looks sensible to me, I’ve had a look in eclair but I was unable to find the findings,
>> here what findings I have in the latest report:
>> https://eclairit.com:8443/job/XEN/Target=ARM64,agent=docker1/lastBuild/eclair/packageName.832204620/fileName.1811675806/
>
> Eclair does not report violations at the definition but rather at the use.
> Check domain_build.c for example to see the reports for 20.7 related to these macros.
Wow yes that’s right, a bit annoying to link the issues! Who knows if there is a mode to make it point out the violation at the definition
like cppcheck.
I’ll review this patch later this week if I manage to find the time
>
> ~Michal
© 2016 - 2026 Red Hat, Inc.