Re: [PATCH] util: fix to get configuration macros in util/mmap-alloc.c

["Liu, Jingqi" <jingqi.liu@intel.com>]

On 3/6/2020 12:40 AM, Peter Maydell wrote:
> On Thu, 5 Mar 2020 at 16:11, Ján Tomko <jtomko@redhat.com> wrote:
>> On a Thursday in 2020, Jingqi Liu wrote:
>>> The CONFIG_LINUX symbol is always not defined in this file.
>>> This fixes that "config-host.h" header file is not included
>>> for getting macros.
>>>
>>> Signed-off-by: Jingqi Liu <jingqi.liu@intel.com>
>>> ---
>>> util/mmap-alloc.c | 2 ++
>>> 1 file changed, 2 insertions(+)
>>>
>>> diff --git a/util/mmap-alloc.c b/util/mmap-alloc.c
>>> index 27dcccd8ec..24c0e380f3 100644
>>> --- a/util/mmap-alloc.c
>>> +++ b/util/mmap-alloc.c
>>> @@ -10,6 +10,8 @@
>>>   * later.  See the COPYING file in the top-level directory.
>>>   */
>>>
>>> +#include "config-host.h"
>>> +
>> According to CODING_STYLE.rst, qemu/osdep.h is the header file
>> that should be included first, before all the other includes.
>>
>> So the minimal fix would be moving qemu/osdep.h up here.
> Yes, osdep must always be first.
>
>>> #ifdef CONFIG_LINUX
>>> #include <linux/mman.h>
>>> #else  /* !CONFIG_LINUX */
> Do we really need this? osdep.h will pull in sys/mman.h
> for you, which should define the MAP_* constants.
>
> Also, you have no fallbmack for "I'm on Linux but the
> system headers don't define MAP_SHARED_VALIDATE or
> MAP_SYNC". Wouldn't it be better to just have
> #ifndef MAP_SYNC
> #define MAP_SYNC 0
> #endif
>
> etc ?
osdep.h pulls in sys/mman.h, which defines the MAP_* constants

except for MAP_SYNC and MAP_SHARED_VALIDATE on Linux.

How about just adding the following code in util/mmap-alloc.c ?

#ifndef MAP_SYNC
#define MAP_SYNC 0x80000
#endif
#ifndef MAP_SYNC
#define MAP_SYNC 0x80000
#endif

#ifndef MAP_SHARED_VALIDATE
#define MAP_SHARED_VALIDATE 0x03
#endif

Thanks,
Jingqi

>
> thanks
> -- PMM