On 11/14/24 07:23, Philippe Mathieu-Daudé wrote:
> On 14/11/24 04:10, Pierrick Bouvier wrote:
>> On 11/13/24 17:12, Philippe Mathieu-Daudé wrote:
>>> 'vaddr' is declared in "exec/vaddr.h".
>>> Include it in order to avoid when refactoring:
>>>
>>> include/exec/translation-block.h:56:5: error: unknown type name
>>> 'vaddr'
>>> 56 | vaddr pc;
>>> | ^
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> ---
>>> include/exec/translation-block.h | 1 +
>>> 1 file changed, 1 insertion(+)
>>>
>>> diff --git a/include/exec/translation-block.h
>>> b/include/exec/translation-block.h
>>> index a6d1af6e9b..b99afb0077 100644
>>> --- a/include/exec/translation-block.h
>>> +++ b/include/exec/translation-block.h
>>> @@ -9,6 +9,7 @@
>>> #include "qemu/thread.h"
>>> #include "exec/cpu-common.h"
>>> +#include "exec/vaddr.h"
>>> #ifdef CONFIG_USER_ONLY
>>> #include "qemu/interval-tree.h"
>>> #endif
>>
>> I'm a bit confused by commit message, but it seems that this series has
>> some commits that will not compile. Is that something acceptable?
>
> Because commits must be bisect-able, that is not acceptable.
>
> I took a lot of care to make this series builable on each step,
> but might have missed something. Can you point me at the
> configuration used and broken patch?
>
> I'll reword the commit description as:
>
> ---
> 'vaddr' type is declared in "exec/vaddr.h".
> "exec/translation-block.h" uses this type without including
> the corresponding header. It works because this header is
> indirectly included, but won't work when the other headers
> are refactored:
>
> [error]
>
> Explitly include "exec/vaddr.h" to avoid such problem in a
> few commits.
> ---
>
> Does it clarify?
>
Yes it's more clear like that. The commit message seemed to imply that
you were fixing build one step at a time.
>> If it's ok,
>> Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>
>
Thanks,
Pierrick