Re: [PATCH 3/6] accel/tcg: Restrict tb_gen_code() from other accelerators

[Claudio Fontana]

On 1/21/21 7:06 AM, Richard Henderson wrote:
> On 1/17/21 11:12 PM, Claudio Fontana wrote:
>> On 1/17/21 5:48 PM, Philippe Mathieu-Daudé wrote:
>>> tb_gen_code() is only called within TCG accelerator,
>>> declare it locally.
>>
>> Is this used only in accel/tcg/cpu-exec.c ? Should it be a static function 
>> there?
> 
> Possibly, but there's a *lot* of code that would have to be moved.  For now,
> queuing a slightly modified version of the patch.
> 
>>> --- a/accel/tcg/user-exec.c
>>> +++ b/accel/tcg/user-exec.c
>>> @@ -28,6 +28,7 @@
>>>  #include "qemu/atomic128.h"
>>>  #include "trace/trace-root.h"
>>>  #include "trace/mem.h"
>>> +#include "internal.h"
> 
> Not needed by this patch.
> 
> 
> r~
> 

Hello,

resurrecting this, and in reference to its commit: 
"c03f041f128301c6a6c32242846be08719cd4fc3",

the name "internal.h" ends up polluting the include paths,
so that when working for example on s390x, including "internal.h" ends up 
including this instead of the file in target/s390x/.

I am not sure what exactly the right solution is, for this specific problem,
and if we should look at the include paths settings in detail,

but in my view calling files just "internal.h" or "internals.h" in general is 
not a good idea.

I can see two issues with this naming:

1) it describes nothing about the actual intended contents, other that they 
should be "internal".
Rather it would be better to know what the file is intended to contain, or we 
end up (as we end up) with very large files containing completely unrelated 
content.

2) we end up with clashes in our include paths if we are not super careful.

Probably in this case, the target/s390x/internal.h could be given another name 
(s390x-internal.h) and then split up in the future (there is a whole bunch of 
unrelated suff).

For accel/tcg/internal.h, maybe renaming it, or removing it altogether could 
both be good options?

Thanks,

Claudio