| Message ID | 20241114011310.3615-4-philmd@linaro.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | exec: Build up 'cputlb.h' and 'ram_addr.h' headers | expand |
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? If it's ok, Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
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? > If it's ok, > Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> >
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
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> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~ > --- > 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
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
'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(+)