On 3/6/2020 12:40 AM, Peter Maydell wrote:
On Thu, 5 Mar 2020 at 16:11, Ján Tomko <jto...@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....@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 ?
Thanks for your review.
The system headers bits/mman.h (which is included by sys/mman.h)
don't define MAP_SYNC and MAP_SHARED_VALIDATE on linux.
They're defined in linux-headers/asm-generic/mman-common.h ( which is
included by linux/mman.h).
Another MAP_* macros are defined in both header files.
As you mentioned, if don't include linux/mman.h, just defines as following:
#ifndef MAP_SYNC
#define MAP_SYNC 0
#endif
The MAP_SYNC is always 0 since MAP_SYNC isn't defined in system header.
Thanks,
Jingqi
thanks
-- PMM
