Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MINGW64 stat defines interfere with FS_CALL macro expansion #624

Closed
rdw-software opened this issue Nov 21, 2022 · 1 comment · Fixed by #625
Closed

MINGW64 stat defines interfere with FS_CALL macro expansion #624

rdw-software opened this issue Nov 21, 2022 · 1 comment · Fixed by #625

Comments

@rdw-software
Copy link
Contributor

When trying to build luv with gcc on MSYS2/MINGW64 (Windows), the uv_fs_stat and uv_fs_fstat APIs aren't exported.

This is likely due to some MINGW defines for stat and fstatmessing up the FS_CALL (and FS_CALL_NORETURN) macros in src/fs.c: Instead of uv_fs_stat the output becomes uv_fs__stat64 and uv_fs_fstat becomes uv_fs__fstat64.

More context: msys2/MINGW-packages#10591


Steps to reproduce:

  1. Start MSYS2 bash with MINGW64 system (toolchain was set up via pacman -S git make mingw-w64-x86_64-gcc ninja mingw-w64-x86_64-cmake --noconfirm)
  2. git clone https://github.com/luvit/luv.git --recursive
  3. cd luv
  4. cmake -S . -B cmakebuild-msys2 -G Ninja -DBUILD_MODULE=OFF -DBUILD_SHARED_LIBS=OFF -DBUILD_STATIC_LIBS=ON
  5. cmake --build cmakebuild-msys2 --clean-first

Expected result: All compilation steps succeed without issues

Actual result: The following warning appears:

[55/59] Building C object CMakeFiles/libluv_a.dir/src/luv.c.obj
In file included from E:/GitHub/Forks/luv/src/luv.c:28:
E:/GitHub/Forks/luv/src/fs.c: In function 'luv_fs_stat':
E:/GitHub/Forks/luv/src/fs.c:433:9: warning: implicit declaration of function 'uv_fs__stat64'; did you mean 'uv_fs_statfs'? [-Wimplicit-function-declaration]
  433 |   ret = uv_fs_##func(data->ctx->loop, req, __VA_ARGS__,   \
      |         ^~~~~~
E:/GitHub/Forks/luv/src/fs.c:433:9: note: in definition of macro 'FS_CALL_NORETURN'
  433 |   ret = uv_fs_##func(data->ctx->loop, req, __VA_ARGS__,   \
      |         ^~~~~~
E:/GitHub/Forks/luv/src/fs.c:629:3: note: in expansion of macro 'FS_CALL'
  629 |   FS_CALL(stat, req, path);
      |   ^~~~~~~
E:/GitHub/Forks/luv/src/fs.c: In function 'luv_fs_fstat':
E:/GitHub/Forks/luv/src/fs.c:433:9: warning: implicit declaration of function 'uv_fs__fstat64'; did you mean 'uv_fs_fstat'? [-Wimplicit-function-declaration]
  433 |   ret = uv_fs_##func(data->ctx->loop, req, __VA_ARGS__,   \
      |         ^~~~~~
E:/GitHub/Forks/luv/src/fs.c:433:9: note: in definition of macro 'FS_CALL_NORETURN'
  433 |   ret = uv_fs_##func(data->ctx->loop, req, __VA_ARGS__,   \
      |         ^~~~~~
E:/GitHub/Forks/luv/src/fs.c:638:3: note: in expansion of macro 'FS_CALL'
  638 |   FS_CALL(fstat, req, file);
      |   ^~~~~~~

Needless to say, the resulting library is broken (undefined reference to `uv_fs__stat64' error when trying to link with it).


To confirm the problem is as I suspected, I applied a naive "hackfix" to fs.c.

Insert before definitions of luv_fs_stat and luv_fs_fstat:

#ifdef stat
#define MINGW_STAT_BACKUP stat
#undef stat
#endif

#ifdef fstat
#define MINGW_FSTAT_BACKUP fstat
#undef fstat
#endif

Then I reset the defines afterwards:

#ifdef MINGW_STAT_BACKUP
#define stat MINGW_STAT_BACKUP
#undef MINGW_STAT_BACKUP
#endif

#ifdef MINGW_FSTAT_BACKUP
#define fstat MINGW_FSTAT_BACKUP
#undef MINGW_FSTAT_BACKUP
#endif

Not exactly a great solution, but it should illustrate the nature of the problem.

Happy to submit a PR if you can suggest a better approach - or maybe this is fine?

@squeek502
Copy link
Member

squeek502 commented Nov 22, 2022

An alternative that avoids any hacks would be to ditch the concatenation in the macro and have it take the full function name, i.e.

- ret = uv_fs_##func(data->ctx->loop, req, __VA_ARGS__,
+ ret = func(data->ctx->loop, req, __VA_ARGS__,

and then at each FS_CALL usage:

- FS_CALL(open, req, path, flags, mode);
+ FS_CALL(uv_fs_open, req, path, flags, mode);

func is only used at that one spot in the FS_CALL macro so there's no real benefit to having it only take the part after the uv_fs_ prefix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants