[PATCH] io/mmap.c: fix musl build on mips64

[PATCH] io/mmap.c: fix musl build on mips64

[Fabrice Fontaine]

musl undefines MAP_SYNC on some architectures such as mips64 since
version 1.1.20 and
https://github.com/ifduyue/musl/commit/9b57db3f958d9adc3b1c7371b5c6723aaee448b7
resulting in the following build failure:

mmap.c: In function 'mmap_f':
mmap.c:196:33: error: 'MAP_SYNC' undeclared (first use in this function); did you mean 'MS_SYNC'?
  196 |                         flags = MAP_SYNC | MAP_SHARED_VALIDATE;
      |                                 ^~~~~~~~
      |                                 MS_SYNC

To fix this build failure, include <sys/mman.h> before the other
includes

Fixes:
 - http://autobuild.buildroot.org/results/3296194907baf7d3fe039f59bcbf595aa8107a28

Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
---
 io/mmap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/io/mmap.c b/io/mmap.c
index 8c048a0a..b8609295 100644
--- a/io/mmap.c
+++ b/io/mmap.c
@@ -4,9 +4,9 @@
  * All Rights Reserved.
  */
 
+#include <sys/mman.h>
 #include "command.h"
 #include "input.h"
-#include <sys/mman.h>
 #include <signal.h>
 #include "init.h"
 #include "io.h"
-- 
2.35.1

Re: [PATCH] io/mmap.c: fix musl build on mips64

[Dave Chinner]

On Mon, Apr 18, 2022 at 10:36:06PM +0200, Fabrice Fontaine wrote:
> musl undefines MAP_SYNC on some architectures such as mips64 since
> version 1.1.20 and
> https://github.com/ifduyue/musl/commit/9b57db3f958d9adc3b1c7371b5c6723aaee448b7
> resulting in the following build failure:
> 
> mmap.c: In function 'mmap_f':
> mmap.c:196:33: error: 'MAP_SYNC' undeclared (first use in this function); did you mean 'MS_SYNC'?
>   196 |                         flags = MAP_SYNC | MAP_SHARED_VALIDATE;
>       |                                 ^~~~~~~~
>       |                                 MS_SYNC
> 
> To fix this build failure, include <sys/mman.h> before the other
> includes
> 
> Fixes:
>  - http://autobuild.buildroot.org/results/3296194907baf7d3fe039f59bcbf595aa8107a28
> 
> Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
> ---
>  io/mmap.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/io/mmap.c b/io/mmap.c
> index 8c048a0a..b8609295 100644
> --- a/io/mmap.c
> +++ b/io/mmap.c
> @@ -4,9 +4,9 @@
>   * All Rights Reserved.
>   */
>  
> +#include <sys/mman.h>
>  #include "command.h"
>  #include "input.h"
> -#include <sys/mman.h>
>  #include <signal.h>
>  #include "init.h"
>  #include "io.h"

I can't see how this makes any difference to the problem, nor can I
see why you are having this issues.

From the configure output:

....
checking for MAP_SYNC... no
....

It is clear that the build has detected that MAP_SYNC does not exist
on this platform, and that means HAVE_MAP_SYNC will not be defined.
That means when xfs_io is built, io/Makefile does not add
-DHAVE_MAP_SYNC to the cflags, and so when io/mmap.c includes
io/io.h -> xfs.h -> linux.h we hit this code:

#ifndef HAVE_MAP_SYNC
#define MAP_SYNC 0
#define MAP_SHARED_VALIDATE 0
#else
#include <asm-generic/mman.h>
#include <asm-generic/mman-common.h>
#endif /* HAVE_MAP_SYNC */

Given that this is the last include in the io/mmap.c file, it should
not matter what musl is doing - this define should be overriding it
completely.

Ooooh.

input.h -> libfrog/projects.h -> xfs.h -> linux.h.

Ok, we've tangled up header includes, and that's why moving the
sys/mman.h header fixes the warning. Right, we need to untangle the
headers.

.... and my build + test rack just had a total internal power
failure of some kind, so that's as far as I can go right now...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH] io/mmap.c: fix musl build on mips64

[Fabrice Fontaine]

Hi,

Were you able to go further in your tests?

Best Regards,

Fabrice


Le mar. 19 avr. 2022 à 01:02, Dave Chinner <david@fromorbit.com> a écrit :
>
> On Mon, Apr 18, 2022 at 10:36:06PM +0200, Fabrice Fontaine wrote:
> > musl undefines MAP_SYNC on some architectures such as mips64 since
> > version 1.1.20 and
> > https://github.com/ifduyue/musl/commit/9b57db3f958d9adc3b1c7371b5c6723aaee448b7
> > resulting in the following build failure:
> >
> > mmap.c: In function 'mmap_f':
> > mmap.c:196:33: error: 'MAP_SYNC' undeclared (first use in this function); did you mean 'MS_SYNC'?
> >   196 |                         flags = MAP_SYNC | MAP_SHARED_VALIDATE;
> >       |                                 ^~~~~~~~
> >       |                                 MS_SYNC
> >
> > To fix this build failure, include <sys/mman.h> before the other
> > includes
> >
> > Fixes:
> >  - http://autobuild.buildroot.org/results/3296194907baf7d3fe039f59bcbf595aa8107a28
> >
> > Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
> > ---
> >  io/mmap.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/io/mmap.c b/io/mmap.c
> > index 8c048a0a..b8609295 100644
> > --- a/io/mmap.c
> > +++ b/io/mmap.c
> > @@ -4,9 +4,9 @@
> >   * All Rights Reserved.
> >   */
> >
> > +#include <sys/mman.h>
> >  #include "command.h"
> >  #include "input.h"
> > -#include <sys/mman.h>
> >  #include <signal.h>
> >  #include "init.h"
> >  #include "io.h"
>
> I can't see how this makes any difference to the problem, nor can I
> see why you are having this issues.
>
> From the configure output:
>
> ....
> checking for MAP_SYNC... no
> ....
>
> It is clear that the build has detected that MAP_SYNC does not exist
> on this platform, and that means HAVE_MAP_SYNC will not be defined.
> That means when xfs_io is built, io/Makefile does not add
> -DHAVE_MAP_SYNC to the cflags, and so when io/mmap.c includes
> io/io.h -> xfs.h -> linux.h we hit this code:
>
> #ifndef HAVE_MAP_SYNC
> #define MAP_SYNC 0
> #define MAP_SHARED_VALIDATE 0
> #else
> #include <asm-generic/mman.h>
> #include <asm-generic/mman-common.h>
> #endif /* HAVE_MAP_SYNC */
>
> Given that this is the last include in the io/mmap.c file, it should
> not matter what musl is doing - this define should be overriding it
> completely.
>
> Ooooh.
>
> input.h -> libfrog/projects.h -> xfs.h -> linux.h.
>
> Ok, we've tangled up header includes, and that's why moving the
> sys/mman.h header fixes the warning. Right, we need to untangle the
> headers.
>
> .... and my build + test rack just had a total internal power
> failure of some kind, so that's as far as I can go right now...
>
> Cheers,
>
> Dave.
> --
> Dave Chinner
> david@fromorbit.com

Re: [PATCH] io/mmap.c: fix musl build on mips64

[Dave Chinner]

On Wed, Apr 27, 2022 at 09:54:02PM +0200, Fabrice Fontaine wrote:
> Hi,
> 
> Were you able to go further in your tests?

I've looked into it a bit more, and this is a string that ends in a
big tangled ball that I do not have time to untangle right now.

Can you please fix the commit message to indicate why moving the
header file avoids the build issue so we have a record of the reason
for the issue occuring in the commit history?

Cheers,

Dave.

-- 
Dave Chinner
david@fromorbit.com