[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] util: fix to get configuration macros in util/mmap-alloc.c
From: |
Michael S. Tsirkin |
Subject: |
Re: [PATCH] util: fix to get configuration macros in util/mmap-alloc.c |
Date: |
Tue, 10 Mar 2020 05:12:36 -0400 |
On Tue, Mar 10, 2020 at 04:58:38PM +0800, Liu, Jingqi wrote:
> On 3/9/2020 9:35 PM, Peter Maydell wrote:
> > On Mon, 9 Mar 2020 at 13:23, Liu, Jingqi <address@hidden> wrote:
> > > On 3/6/2020 12:40 AM, Peter Maydell wrote:
> > > > On Thu, 5 Mar 2020 at 16:11, Ján Tomko <address@hidden> 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 <address@hidden>
> > > > > > ---
> > > > > > 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.
> > Why not? Is this just "not yet in the version of glibc
> > we're using", or is it a bug/missed feature in glibc
> > that needs to be addressed there ?
>
> I'm using the version 2.27 of glibc.
>
> I downloaded the version 2.28 of glibc source for compilation and
> installation.
>
> I found MAP_SYNC and MAP_SHARED_VALIDATE are defined in this version.
>
> Seems it's older glibc version issue.
>
> >
> > > How about just adding the following code in util/mmap-alloc.c ?
> > > #ifndef MAP_SYNC
> > > #define MAP_SYNC 0x80000
> > > #endif
> > >
> > > #ifndef MAP_SHARED_VALIDATE
> > > #define MAP_SHARED_VALIDATE 0x03
> > > #endif
> > You don't want to do that for non-Linux systems, so there
> > you need to fall back to defining them to be 0.
> >
> > Are there any systems (distros) where the standard system
> > sys/mman.h does not define these new MAP_* constants but we
> > still really really need to use them? If not, then we
> > could just have the fallback-to-0 fallback everywhere.
>
> Good point.
>
> So as you mentioned, it would be better to just have the following code:
>
> #ifndef MAP_SYNC
> #define MAP_SYNC 0
> #endif
>
> #ifndef MAP_SHARED_VALIDATE
> #define MAP_SHARED_VALIDATE 0
> #endif
Won't this defeat the purpose of MAP_SHARED_VALIDATE?
We really have linux-headers/linux/mman.h for exactly this purpose.
> Thanks,
>
> Jingqi
>
> >
> > thanks
> > -- PMM
- [PATCH] util: fix to get configuration macros in util/mmap-alloc.c, Jingqi Liu, 2020/03/05
- Re: [PATCH] util: fix to get configuration macros in util/mmap-alloc.c, Ján Tomko, 2020/03/05
- Re: [PATCH] util: fix to get configuration macros in util/mmap-alloc.c, Peter Maydell, 2020/03/05
- Re: [PATCH] util: fix to get configuration macros in util/mmap-alloc.c, Liu, Jingqi, 2020/03/05
- Re: [PATCH] util: fix to get configuration macros in util/mmap-alloc.c, Liu, Jingqi, 2020/03/09
- Re: [PATCH] util: fix to get configuration macros in util/mmap-alloc.c, Peter Maydell, 2020/03/09
- Re: [PATCH] util: fix to get configuration macros in util/mmap-alloc.c, Liu, Jingqi, 2020/03/10
- Re: [PATCH] util: fix to get configuration macros in util/mmap-alloc.c,
Michael S. Tsirkin <=
- Re: [PATCH] util: fix to get configuration macros in util/mmap-alloc.c, Liu, Jingqi, 2020/03/10
- Re: [PATCH] util: fix to get configuration macros in util/mmap-alloc.c, Peter Maydell, 2020/03/11
- Re: [PATCH] util: fix to get configuration macros in util/mmap-alloc.c, Eduardo Habkost, 2020/03/11
Re: [PATCH] util: fix to get configuration macros in util/mmap-alloc.c, Liu, Jingqi, 2020/03/05