MC 4.8.22 doesn't compile on AIX 7.2

[dp]

Hi there,

The compilation fails on AIX 7.2 (7200-03-01-1838) with the following error messages:

make[3]: Entering directory '/home/dp/rpmbuild/BUILD/mc-4.8.22/src/filemanager'
  CC       achown.lo
achown.c: In function 'chl_callback':
achown.c:491:9: warning: this statement may fall through [-Wimplicit-fallthrough=]
         switch (parm)
         ^~~~~~
achown.c:505:5: note: here
     default:
     ^~~~~~~
  CC       boxes.lo
  CC       chmod.lo
  CC       chown.lo
  CC       cmd.lo
  CC       command.lo
In function 'examine_cd',
    inlined from 'do_cd_command' at command.c:440:16:
command.c:172:25: warning: 'strncpy' specified bound depends on the length of the source argument [-Wstringop-overflow=]
                         strncpy (r, t, tlen + 1);
                         ^~~~~~~~~~~~~~~~~~~~~~~~
command.c: In function 'do_cd_command':
command.c:168:28: note: length computed here
                     tlen = strlen (t);
                            ^~~~~~~~~~
  CC       dir.lo
  CC       ext.lo
  CC       file.lo
file.c: In function 'get_times':
file.c:899:17: error: incompatible types when assigning to type 'struct timespec' from type 'st_timespec_t' {aka 'const struct st_timespec'}
     (*times)[0] = sb->st_atim;
                 ^
file.c:900:17: error: incompatible types when assigning to type 'struct timespec' from type 'st_timespec_t' {aka 'const struct st_timespec'}
     (*times)[1] = sb->st_mtim;
                 ^
make[3]: *** [Makefile:604: file.lo] Error 1
make[3]: Leaving directory '/home/dp/rpmbuild/BUILD/mc-4.8.22/src/filemanager'
make[2]: *** [Makefile:738: all-recursive] Error 1
make[2]: Leaving directory '/home/dp/rpmbuild/BUILD/mc-4.8.22/src'
make[1]: *** [Makefile:625: all-recursive] Error 1
make[1]: Leaving directory '/home/dp/rpmbuild/BUILD/mc-4.8.22'
make: *** [Makefile:553: all] Error 2

The full listing is attached.

[zaytsev]

Sounds like compilation on AIX got busted by PASE changes :( Does any of you have access to AIX / can help with that?

[andrew_b]

Warning are fixed in the 3955_cleanup branch.

I guess the fix of compilation error for AIX should be the same as fix for IBM i: [a45337672be6f32df2a598f3fdc03e3c0b8f53ac].

[zaytsev]

dp, could you please try the attached patch out? thanks!

[dp]

Replying to zaytsev:

Sounds like compilation on AIX got busted by PASE changes :( Does any of you have access to AIX / can help with that?

Yes, of course. I can help you. I have an unlimited access to AIX.

[dp]

Replying to zaytsev:

dp, could you please try the attached patch out? thanks!

I have done it, but the compilation failed again.((
Please find the new attached file compilation2.log.

[andrew_b]

Replying to dp:

I have done it, but the compilation failed again.((
Please find the new attached file compilation2.log.

There is no difference between logs.
Did you run autoreconf after apply patch to configure.ac?

[dp]

Replying to andrew_b:

Replying to dp:

I have done it, but the compilation failed again.((
Please find the new attached file compilation2.log.

There is no difference between logs.
Did you run autoreconf after apply patch to configure.ac?

Sorry, no. I didn't know it. This time mc has been successfully compiled.

[andrew_b]

Branch: 3960_aix
changeset:56320a331498bda962bc13d293c8274d311c05bb

[andrew_b]

Merged to master: [56320a331498bda962bc13d293c8274d311c05bb].

[zaytsev]

GCC has AIX 7.1 and AIX 7.3 machines. Sadly no IBM i access. But I suspect that the problem is/was the same as on macOS. They implemented nanosecond access via st_atimespec instead of st_atim, so instead of this patch we should check whether this is true and maybe still implement the ultimate support.

[zaytsev]

So I think I've got mc @ 4542_nanoseconds_cleanup to build on AIX 7.3 with glib-2.43.92 and latest gettext / libffi / slang.

It even seems to even mostly work correctly in as far as timestamps are concerned, which is impressive. The original patches were wrong, the kernel actually support everything almost correctly, only structure name is different for binary compatibility reasons (!?), so barring a hard cast one could just assign field by field and be done with it.

If anyone has interest to test on AIX or PASE, you are most welcome. I could also get my hands on AIX 7.1, but the machines are so slow and AIX is so painful to work with.

The magic sauce for mc is (it finds termcap, but can't link?):

GLIB_LIBS="-L$HOME/opt/glib/lib -lglib-2.0" \
GLIB_CFLAGS="-I$HOME/opt/glib/include/glib-2.0 -I$HOME/opt/glib/lib/glib-2.0/include" \
SLANG_LIBS="-L$HOME/opt/slang/lib -lslang" \
SLANG_CFLAGS=-I$HOME/opt/slang/include \
  ../configure --prefix=$HOME/opt/mc

Slang must be compiled with make static and make install-static.

LDFLAGS=-static ./configure --prefix=$HOME/opt/slang
gmake static
gmake install-static

glib-2.43.92 (need to patch out -werror and codegen stuff):

LIBFFI_LIBS=$HOME/opt/libffi/lib \
LIBFFI_CFLAGS=-I$HOME/opt/libffi/include \
LDFLAGS=-L$HOME/opt/gettext/lib \
CPPFLAGS=-I$HOME/opt/gettext/include \
  ../configure --disable-static --enable-shared --prefix=$HOME/opt/glib

libffi and gettext are ok. Next time maybe still a good idea to install pkg-config...

P.S. Broken termcap detection is fixed on master.

[zaytsev]

AIX can show nanosecond timestamps with ls -als --full-time if GNU ls is available.

[zaytsev]

Interesting remaining warnings: probably some size_t type of thing? Not sure how to fix this.

../../../src/filemanager/cmd.c: In function 'compare_files':
../../../src/filemanager/cmd.c:224:36: warning: 'memcmp' specified size 4294967295 exceeds maximum object size 2147483647 [-Wstringop-overflow=]
  224 |             result = (n1 != n2) || memcmp (buf1, buf2, n1);
      |                                    ^~~~~~~~~~~~~~~~~~~~~~~

The rest looks really surprisingly good. Just a few warnings from GNU code and some gettext stuff.

[andrew_b]

Replying to zaytsev:

Interesting remaining warnings: probably some size_t type of thing? Not sure how to fix this.

Probably change int to ssize_t (as read(2) returns ssize_t), initialization and size_t/ssize_t casting:

src/filemanager/cmd.c

@@ -210 +210 @@
 #else
             /* Don't have mmap() :( Even more ugly :) */
             char buf1[BUFSIZ], buf2[BUFSIZ];
-            int n1, n2;
+            ssize_t n1 = 0;
+            ssize_t n2 = 0;
 
             rotate_dash (TRUE);
             do
@@ -220 +221 @@
                 while ((n2 = read (file2, buf2, sizeof (buf2))) == -1 && errno == EINTR)
                     ;
             }
-            while (n1 == n2 && n1 == sizeof (buf1) && memcmp (buf1, buf2, sizeof (buf1)) == 0);
-            result = (n1 != n2) || memcmp (buf1, buf2, n1);
+            while (n1 == n2 && n1 == (ssize_t) sizeof (buf1)
+                   && memcmp (buf1, buf2, sizeof (buf1)) == 0);
+            result = (n1 != n2) || memcmp (buf1, buf2, (size_t) n1);
 #endif /* !HAVE_MMAP */
             close (file2);
         }

[zaytsev]

So this problem is caused by GCC bug, but I would suggest the following patch for clarity anyways:

diff --git a/src/filemanager/cmd.c b/src/filemanager/cmd.c
index 70602db04..133c8a168 100644
--- a/src/filemanager/cmd.c
+++ b/src/filemanager/cmd.c
@@ -210,7 +210,7 @@ compare_files (const vfs_path_t *vpath1, const vfs_path_t *vpath2, off_t size)
 #else
             /* Don't have mmap() :( Even more ugly :) */
             char buf1[BUFSIZ], buf2[BUFSIZ];
-            int n1, n2;
+            ssize_t n1, n2;
 
             rotate_dash (TRUE);
             do

​https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100571

But the real problem is that our mmap-code is not used, and instead a fallback is used. There is an unsafe way to override it, but I would delete it:

diff --git a/configure.ac b/configure.ac
index a18defa63..7f99554e2 100644
--- a/configure.ac
+++ b/configure.ac
@@ -317,19 +317,6 @@ dnl replacing lstat with statlstat on sco makes it more portable between
 dnl sco clones
 AC_CHECK_FUNCS(statlstat)
 
-dnl Overriding mmap support.  This has to be before AC_FUNC_MMAP is used.
-dnl We use only part of the functionality of mmap, so on AIX,
-dnl it's possible to use mmap, even if it doesn't pass the autoconf test.
-AC_ARG_WITH([mmap],
-       AS_HELP_STRING([--with-mmap], [Use the mmap call @<:@yes if found@:>@]))
-if test x$with_mmap != xno; then
-    if test x$with_mmap = x; then
-       AC_FUNC_MMAP
-    else
-       AC_DEFINE(HAVE_MMAP, 1)
-    fi
-fi
-
 mc_GET_FS_INFO

It's only used in file comparison nowadays, and even though mmap version is probably more performant, I'm not sure if that really matters.

The problem here is that the Autoconf test fails:

|   if (data2 != mmap (data2, pagesize, PROT_READ | PROT_WRITE,                                                                  
|                    MAP_PRIVATE | MAP_FIXED, fd, 0L))                                                                           
|     return 10;                                                                                                                 

But it's not clear to me whether the test is bad, or AIX has been broken for decades. I've asked on the autoconf list, but probably it's on pre-moderation.

[zaytsev]

​https://lists.gnu.org/archive/html/autoconf/2024-07/msg00006.html

[zaytsev]

So it seems that the facts are as follows:

1. mmap macro was introduced to autoconf by grep people and does nothing wrong
2. mmap on AIX has been broken at least in a sense of MAP_FIXED support for decades
3. grep later got rid of mmap, because in the end it didn't make things any faster
4. mc used mmap in the viewer, for file comparison and in other places
5. mc provided --with-mmap to force using broken mmap implementations
6. mc viewer is now rewritten and remaining code has an okayish fallback

Having that said, I would just remove mmap code, the override and whatever else has to do with that and can be removed. Then we can close this ticket... for now. Sounds good?

[andrew_b]

Yes.

[zaytsev]

Branch: 3960_remove_mmap
Initial changeset:5b335920aa59a8a120914899b71a752acb29f1d9

I hope IO_BUFSIZE is okay for the stack...?

Also, I took time to update the documentation: mmap isn't used in the viewer, so I removed that. Some other advice is either obsolete or wrong. So I deleted extra files and updated the links. Also I have removed some general tutorial text on how to use autotools, and replaced it with specific advice for bootstrapping and porting. I hope that this is uncontroversial.

[zaytsev]

Incidentally, fixed #3983.