Re: [Qemu-devel] [PATCH] translate-all: wrapped map_exec() in #ifdef

[Michael Tokarev <mjt@tls.msk.ru>]

03.11.2014 11:18, SeokYeon Hwang wrote:
>> -----Original Message-----
>> From: Peter Maydell [mailto:peter.maydell@linaro.org]
>> Sent: Saturday, November 01, 2014 3:30 AM
>> To: SeokYeon Hwang
>> Cc: QEMU Developers
>> Subject: Re: [Qemu-devel] [PATCH] translate-all: wrapped map_exec() in
>> #ifdef
>>
>> On 31 October 2014 04:59, SeokYeon Hwang <syeon.hwang@samsung.com> wrote:
>>> Moved map_exec() and wrapped it in #ifdef to avoid "-Wunused-function"
>> on clang 3.4 or later.
>>>
>>> Signed-off-by: SeokYeon Hwang <syeon.hwang@samsung.com>
>>
>> I had this kind of on my todo list too, but I didn't much like the nested
>> ifdefs which are really only because of what the different implementations
>> of alloc_code_gen_buffer() happen to do. I think it would be more robust
>> to just mark the functions with the 'unused' attribute instead of relying
>> on 'inline' to implicitly do this for us:
>>
>> --- a/translate-all.c
>> +++ b/translate-all.c
>> @@ -270,14 +270,14 @@ bool cpu_restore_state(CPUState *cpu, uintptr_t
>> retaddr)  }
>>
>>  #ifdef _WIN32
>> -static inline void map_exec(void *addr, long size)
>> +static __attribute__((unused)) void map_exec(void *addr, long size)
>>  {
>>      DWORD old_protect;
>>      VirtualProtect(addr, size,
>>                     PAGE_EXECUTE_READWRITE, &old_protect);  }

In which case this function isn't used on windows?  I mean, is it really
necessary to mark it as unused for win32 case?

>> #else
>> -static inline void map_exec(void *addr, long size)
>> +static __attribute__((unused)) void map_exec(void *addr, long size)
>>  {
>>      unsigned long start, end, page_size;

How about this instead:

--- a/translate-all.c
+++ b/translate-all.c
@@ -276,7 +276,7 @@ static inline void map_exec(void *addr, long size)
     VirtualProtect(addr, size,
                    PAGE_EXECUTE_READWRITE, &old_protect);
 }
-#else
+#elif !defined(USE_MMAP)
 static inline void map_exec(void *addr, long size)
 {
     unsigned long start, end, page_size;

? (Untested, but just to show an idea)... ;)

Thanks,

/mjt