missing prototypes warnings when building on windows using msys2

[FractalU]

I've manually built luafilesystem on window using msys2, a unix like environment on windows similar to cygwin. From gcc I get the following warnings.

src/lfs.c:171:5: warning: no previous prototype for 'lfs_win32_pusherror' [-Wmissing-prototypes]
  171 | int lfs_win32_pusherror(lua_State * L)
      |
src/lfs.c:184:8: warning: no previous prototype for 'windowsToUnixTime' [-Wmissing-prototypes]
  184 | time_t windowsToUnixTime(FILETIME ft)
      |
src/lfs.c:192:5: warning: no previous prototype for 'lfs_win32_lstat' [-Wmissing-prototypes]
  192 | int lfs_win32_lstat(const char *path, STAT_STRUCT * buffer)
      |

I guess all the functions mentioned in the warnings should be declared as static. Below is the proposed change.

In lfs.c at line 169

#ifdef _WIN32

static int lfs_win32_pusherror(lua_State * L)
{
  int en = GetLastError();
  lua_pushnil(L);
  if (en == ERROR_FILE_EXISTS || en == ERROR_SHARING_VIOLATION)
    lua_pushstring(L, "File exists");
  else
    lua_pushstring(L, strerror(en));
  return 2;
}

#define TICKS_PER_SECOND 10000000
#define EPOCH_DIFFERENCE 11644473600LL
static time_t windowsToUnixTime(FILETIME ft)
{
  ULARGE_INTEGER uli;
  uli.LowPart = ft.dwLowDateTime;
  uli.HighPart = ft.dwHighDateTime;
  return (time_t) (uli.QuadPart / TICKS_PER_SECOND - EPOCH_DIFFERENCE);
}

static int lfs_win32_lstat(const char *path, STAT_STRUCT * buffer)
{
  WIN32_FILE_ATTRIBUTE_DATA win32buffer;
  if (GetFileAttributesEx(path, GetFileExInfoStandard, &win32buffer)) {
    if (!(win32buffer.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT)) {
      return STAT_FUNC(path, buffer);
    }
    buffer->st_mode = _S_IFLNK;
    buffer->st_dev = 0;
    buffer->st_ino = 0;
    buffer->st_nlink = 0;
    buffer->st_uid = 0;
    buffer->st_gid = 0;
    buffer->st_rdev = 0;
    buffer->st_atime = windowsToUnixTime(win32buffer.ftLastAccessTime);
    buffer->st_mtime = windowsToUnixTime(win32buffer.ftLastWriteTime);
    buffer->st_ctime = windowsToUnixTime(win32buffer.ftCreationTime);
    buffer->st_size = 0;
    return 0;
  } else {
    return 1;
  }
}

#endif

Declare all these functions as static.

[FractalU]

@hishamhm Do you prefer a pull request here as well. This bug report is standing for more than a year now. The solution is really simple. Add static to these three functions lfs_win32_pusherror, windowsToUnixTime, lfs_win32_lstat in order to fix the missing prototypes warnings under windows.

[Tieske]

I think it would be good to add to CI first, showing the failures. Here's some prior art; https://github.com/lunarmodules/luasystem/blob/master/.github/workflows/build.yml

It uses both MSVC as well as the MinGW/gcc toolchains (for PuC and LuaJIT Lua's respectively)

[FractalU]

Don't really know whether MSVC shows a -Wmissing-prototypes warning or similar as well. If it doesn't then this issue can easily be overlooked. According to workflows/build.yml MinGW/gcc toolchain is only being used for the Luajit build.

But looking from a logical perspective. All functions in lfs.c which aren't being declared with LFS_EXPORT are being declared as static with the exception of the following three windows specific functions,

lfs_win32_pusherror
windowsToUnixTime
lfs_win32_lstat

Yet, all these three functions are fully local in lfs.c and don't have a prototype in lfs.h. So why not declare them as static, so that every function in lfs.c which isn't being declared with LFS_EXPORT is being declared as static. Then, only the functions which are being declared with LFS_EXPORT have a prototype in lfs.h.

[hishamhm]

@hishamhm Do you prefer a pull request here as well.

@FractalU yes please, especially since I don't have a way to test on Windows locally.

According to workflows/build.yml MinGW/gcc toolchain is only being used for the Luajit build.

What the comments there mean is that the only combinations tested are Windows+Lua 5.4+MSVC and Windows+LuaJIT+MinGW/gcc, but then luarocks make runs with whatever compiler is being used. So I think that latter combination would still trigger the warnings you're referring to.

But looking from a logical perspective...

Yes, it does make sense!

[FractalU]

Alright, made a pull request here as well.