#188 compatglob fails to build with newer compilers

[Michael Orlitzky]

When building against musl libc (and probably any other libc where glob is missing GLOB_ALTDIRFUNC) and using GCC 15.x, the build fails due to the incompatible pointer types for the two readdir functions. GCC-15 makes this an error even when -Werror is disabled:

compatglob.c: In function 'glob3':
compatglob.c:585:29: error: assignment to 'struct dirent * (*)(void)' from incompatible pointer type 'struct dirent * (*)(void *)' [-Wincompatible-pointer-types]
  585 |                 readdirfunc = pglob->gl_readdir;
      |                             ^
compatglob.c:587:29: error: assignment to 'struct dirent * (*)(void)' from incompatible pointer type 'struct dirent * (*)(DIR *)' [-Wincompatible-pointer-type]
  587 |                 readdirfunc = readdir;
      |                             ^
In file included from compatglob.c:75:
/usr/include/dirent.h:27:16: note: 'readdir' declared here
   27 | struct dirent *readdir(DIR *);
      |                ^~~~~~~

Hacking the prototype and using a cast fixes the build...

--- a/compat/lib/compatglob.c
+++ b/compat/lib/compatglob.c
@@ -562,7 +562,7 @@

     * and dirent.h as taking pointers to differently typed opaque
     * structures.
     */
-   struct dirent *(*readdirfunc)();
+   struct dirent *(*readdirfunc)(void *);

    *pathend = EOS;
    errno = 0;
@@ -584,7 +584,7 @@
    if (pglob->gl_flags & GLOB_ALTDIRFUNC)
        readdirfunc = pglob->gl_readdir;
    else

-       readdirfunc = readdir;
+       readdirfunc = (struct dirent * (*)(void *))readdir;
    while ((dp = (*readdirfunc)(dirp))) {
        u_char *sc;
        Char *dc;

but my knowledge of the C standard isn't strong enough to know if this is reasonable or a time bomb.

I don't have any other good ideas for fixing it, particularly since I don't know how well the remaining extended glob features are supported. (If ALTDIRFUNC is all that is missing, you could use compatglob only when ALTDIRFUNC is needed.)

[Tim Woodall]

Thank you for this report.

I've quickly hacked together what I think is the correct fix in the branch v0.4b54-wip

This commit in particular I believe will fix this (and the endian includes issues)
https://sourceforge.net/p/dump/code/ci/721f0e1b65af1ec1de9c959c92281d1a7197aaae/

I haven't actually tested with musl which is still on my "I wish I had time to get this working" list but I think iwyu is now clean so hopefully I'm not relying on any debian/gcc specific includes.

I very much doubt I'll merge that commit exactly as it is now, I've done this in a hurry and it probably could do with some cleanup work :-) But if it works for you (at least the bits that you need) then as least I know I'm on the right track.

[Michael Orlitzky]

dump itself builds OK on that branch, but faketape/dump-info.cpp is missing sys/stat.h:

g++ -DHAVE_CONFIG_H -I. -I..   -D_USE_BSD_SIGNAL -D_PATH_DUMPDATES=\"/usr/local/etc/dumpdates\" -D_DUMP_VERSION=\"0.4b53\" -D_PATH_RMT=\"/etc/rmt\"  -O3 -pipe -mabi=lp64d -march=rv64gc -W -Wall -std=c++20 -MT dump-info.o -MD -MP -MF .deps/dump-info.Tpo -c -o dump-info.o dump-info.cpp
dump-info.cpp: In function 'std::string getmode(uint16_t)':
dump-info.cpp:194:23: error: 'S_IFMT' was not declared in this scope
  194 |         switch(mode & S_IFMT) {
      |                       ^~~~~~
dump-info.cpp:195:22: error: 'S_IFDIR' was not declared in this scope
  195 |                 case S_IFDIR: result += " directory"; break;
      |                      ^~~~~~~

and bswap_header.cpp is missing... something else:

In file included from bswap_header.cpp:42:
/usr/include/ext2fs/ext2_ext_attr.h:19:9: error: '__u32' does not name a type
   19 |         __u32   h_magic;        /* magic number for identification */
      |         ^~~~~

[Tim Woodall]

Another try - I've force pushed that branch. sys/stat.h was just a iwyu problem, I've added an extra mapping to iwyu.imp so hopefully it won't recur. bswap_header.cpp was an ext2 header weirdness - ext2_ext_attr.h doesn't pull in the right headers so it needed reordering. On a debian build there's another __u32 being pulled in first which is why it worked for me.

The entire branch includes work to fix what I think is an s390x endianness issue but it hasn't been tested yet except via a very convoluted qemu plus patched libfuse on amd64.

[Michael Orlitzky]

Looks good now! I re-ran all of the non-destructive tests you suggested to me on https://sourceforge.net/p/dump/feature-requests/16/, and everything appears to be working.

One last request (not pressing) while I am being annoying: on Gentoo, we are disabling link-time optimization (LTO) for dump because it fails to build with CFLAGS="-Werror=odr -Werror=lto-type-mismatch -Werror=strict-aliasing" all of which tend to indicate problems that could confuse LTO. If this is easy to fix, it may avoid some surprises, especially for users who blindly enable LTO without those safeguards.

extern.h:126:17: error: type of 'rmtread' does not match original declaration [-Werror=lto-type-mismatch]
  126 | int             rmtread (const char *, int);
      |                 ^
../common/dumprmt.c:318:1: note: type mismatch in parameter 2
  318 | rmtread(char *buf, size_t count)
      | ^
../common/dumprmt.c:318:1: note: type 'size_t' should match type 'int'
../common/dumprmt.c:318:1: note: 'rmtread' was previously declared here
../common/dumprmt.c:318:1: note: code may be misoptimized unless '-fno-strict-aliasing' is used

[Tim Woodall]

Oh, that was a bad one! I guess I need a newer compiler to reproduce.
I've fixed it on the v0.4b54-wip branch and cleaned that up. If that works for you I'll merge it in as a new release.
The 0.4b55-wip branch has the other stuff on it (plus what I was working on today) but this definitely isn't ready for release yet.

[Michael Orlitzky]

Yep that fixed it, thanks again.

[Tim Woodall]

0.4b54 is now released.

[Tim Woodall]

0.4b54 is now released.