Use fopen64, fix segfault in fseek on arm #29 (#54)

* Use fopen64, fix segfault in fseek on arm #29 * Build fails on OS X, use specific ARMv7 check * Use large file test from the OpenJPG project * With the flag _FILE_OFFSET_BITS set to 64 (via the openjpg cmake file), regular fopen should be good in all cases. --------- Co-authored-by: R. van Elst <[email protected]>
lenticularis39 · May 9, 2023 · 7543f2d · 7543f2d
1 parent a778050
commit 7543f2d
Show file tree

Hide file tree

Showing 8 changed files with 204 additions and 13 deletions.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -24,13 +24,17 @@ endif()
 find_package(Threads REQUIRED)
 target_link_libraries(axpbox Threads::Threads)
 
+# Path to additional CMake modules
+set(CMAKE_MODULE_PATH ${CMAKE_SOURCE_DIR}/cmake)
+
 # Configuration options
 include(CheckSymbolExists)
 include(CheckIncludeFile)
 include(CheckLibraryExists)
 include(CheckTypeSize)
 include(CheckIncludeFiles)
 include(TestBigEndian)
+include(CheckLargeFiles)
 
 TEST_BIG_ENDIAN(IS_BIG_ENDIAN)
 if(IS_BIG_ENDIAN)
@@ -114,6 +118,9 @@ check_symbol_exists(_stricmp "string.h" HAVE__STRICMP)
 check_include_file("pcap.h" HAVE_PCAP)
 check_include_file("SDL/SDL.h" HAVE_SDL)
 check_include_file("X11/X.h" HAVE_X11)
+# Large file support (fopen64 / disk files > 2 GB)
+AXPBOX_TEST_LARGE_FILES(HAVE_LARGE_FILES)
+
 
 
 if (DISABLE_PCAP STREQUAL "yes")
@@ -163,6 +170,12 @@ if (NOT CMAKE_GENERATOR MATCHES "Visual Studio")
     endif()
 endif()
 
+if(HAVE_LARGE_FILES)
+    message(STATUS "Large file support enabled")
+else()
+    message(WARNING "Building without large file graphics support")
+endif()
+
 find_package(Git)
 if (GIT_FOUND)
     execute_process(COMMAND

diff --git a/cmake/CheckLargeFiles.cmake b/cmake/CheckLargeFiles.cmake
@@ -0,0 +1,132 @@
+# Via: https://github.com/uclouvain/openjpeg/blob/master/cmake/TestLargeFiles.cmake
+# - Define macro to check large file support
+#
+#  AXPBOX_TEST_LARGE_FILES(VARIABLE)
+#
+#  VARIABLE will be set to true if off_t is 64 bits, and fseeko/ftello present.
+#  This macro will also defines the necessary variable enable large file support, for instance
+#  _LARGE_FILES
+#  _LARGEFILE_SOURCE
+#  _FILE_OFFSET_BITS 64
+#  AXPBOX_HAVE_FSEEKO
+#
+#  Adapted from Gromacs project (http://www.gromacs.org/)
+#  by Julien Malik
+#
+
+macro(AXPBOX_TEST_LARGE_FILES VARIABLE)
+    if(NOT DEFINED ${VARIABLE})
+
+        # On most platforms it is probably overkill to first test the flags for 64-bit off_t,
+        # and then separately fseeko. However, in the future we might have 128-bit filesystems
+        # (ZFS), so it might be dangerous to indiscriminately set e.g. _FILE_OFFSET_BITS=64.
+
+        message(STATUS "Checking for 64-bit off_t")
+
+        # First check without any special flags
+        try_compile(FILE64_OK "${PROJECT_BINARY_DIR}"
+                "${PROJECT_SOURCE_DIR}/cmake/TestFileOffsetBits.c")
+        if(FILE64_OK)
+            message(STATUS "Checking for 64-bit off_t - present")
+        endif()
+
+        if(NOT FILE64_OK)
+            # Test with _FILE_OFFSET_BITS=64
+            try_compile(FILE64_OK "${PROJECT_BINARY_DIR}"
+                    "${PROJECT_SOURCE_DIR}/cmake/TestFileOffsetBits.c"
+                    COMPILE_DEFINITIONS "-D_FILE_OFFSET_BITS=64" )
+            if(FILE64_OK)
+                message(STATUS "Checking for 64-bit off_t - present with _FILE_OFFSET_BITS=64")
+                set(_FILE_OFFSET_BITS 64)
+            endif()
+        endif()
+
+        if(NOT FILE64_OK)
+            # Test with _LARGE_FILES
+            try_compile(FILE64_OK "${PROJECT_BINARY_DIR}"
+                    "${PROJECT_SOURCE_DIR}/cmake/TestFileOffsetBits.c"
+                    COMPILE_DEFINITIONS "-D_LARGE_FILES" )
+            if(FILE64_OK)
+                message(STATUS "Checking for 64-bit off_t - present with _LARGE_FILES")
+                set(_LARGE_FILES 1)
+            endif()
+        endif()
+
+        if(NOT FILE64_OK)
+            # Test with _LARGEFILE_SOURCE
+            try_compile(FILE64_OK "${PROJECT_BINARY_DIR}"
+                    "${PROJECT_SOURCE_DIR}/cmake/TestFileOffsetBits.c"
+                    COMPILE_DEFINITIONS "-D_LARGEFILE_SOURCE" )
+            if(FILE64_OK)
+                message(STATUS "Checking for 64-bit off_t - present with _LARGEFILE_SOURCE")
+                set(_LARGEFILE_SOURCE 1)
+            endif()
+        endif()
+
+
+        #if(NOT FILE64_OK)
+        #    # now check for Windows stuff
+        #    try_compile(FILE64_OK "${PROJECT_BINARY_DIR}"
+        #                "${PROJECT_SOURCE_DIR}/cmake/TestWindowsFSeek.c")
+        #    if(FILE64_OK)
+        #        message(STATUS "Checking for 64-bit off_t - present with _fseeki64")
+        #        set(HAVE__FSEEKI64 1)
+        #    endif()
+        #endif()
+
+        if(NOT FILE64_OK)
+            message(STATUS "Checking for 64-bit off_t - not present")
+        endif()
+
+        set(_FILE_OFFSET_BITS ${_FILE_OFFSET_BITS} CACHE INTERNAL "Result of test for needed _FILE_OFFSET_BITS=64")
+        set(_LARGE_FILES      ${_LARGE_FILES}      CACHE INTERNAL "Result of test for needed _LARGE_FILES")
+        set(_LARGEFILE_SOURCE ${_LARGEFILE_SOURCE} CACHE INTERNAL "Result of test for needed _LARGEFILE_SOURCE")
+
+        # Set the flags we might have determined to be required above
+        configure_file("${PROJECT_SOURCE_DIR}/cmake/TestLargeFiles.c.cmake.in"
+                "${PROJECT_BINARY_DIR}${CMAKE_FILES_DIRECTORY}/TestLargeFiles.c")
+
+        message(STATUS "Checking for fseeko/ftello")
+
+        # Test if ftello/fseeko are	available
+        try_compile(FSEEKO_COMPILE_OK
+                "${PROJECT_BINARY_DIR}"
+                "${PROJECT_BINARY_DIR}${CMAKE_FILES_DIRECTORY}/TestLargeFiles.c")
+
+        if(FSEEKO_COMPILE_OK)
+            message(STATUS "Checking for fseeko/ftello - present")
+        endif()
+
+        if(NOT FSEEKO_COMPILE_OK)
+            # glibc 2.2 needs _LARGEFILE_SOURCE for fseeko (but not for 64-bit off_t...)
+            try_compile(FSEEKO_COMPILE_OK
+                    "${PROJECT_BINARY_DIR}"
+                    "${PROJECT_BINARY_DIR}${CMAKE_FILES_DIRECTORY}/TestLargeFiles.c"
+                    COMPILE_DEFINITIONS "-D_LARGEFILE_SOURCE" )
+
+            if(FSEEKO_COMPILE_OK)
+                message(STATUS "Checking for fseeko/ftello - present with _LARGEFILE_SOURCE")
+                set(_LARGEFILE_SOURCE ${_LARGEFILE_SOURCE} CACHE INTERNAL "Result of test for needed _LARGEFILE_SOURCE")
+            endif()
+        endif()
+
+        if(FSEEKO_COMPILE_OK)
+            set(AXPBOX_HAVE_FSEEKO ON CACHE INTERNAL "Result of test for fseeko/ftello")
+        else()
+            message(STATUS "Checking for fseeko/ftello - not found")
+            set(AXPBOX_HAVE_FSEEKO OFF CACHE INTERNAL "Result of test for fseeko/ftello")
+        endif()
+
+        if(FILE64_OK AND FSEEKO_COMPILE_OK)
+            message(STATUS "Large File support - found")
+            set(${VARIABLE} ON CACHE INTERNAL "Result of test for large file support")
+        else()
+            message(STATUS "Large File support - not found")
+            set(${VARIABLE} OFF CACHE INTERNAL "Result of test for large file support")
+        endif()
+
+    endif()
+endmacro()
+
+
+
diff --git a/cmake/TestFileOffsetBits.c b/cmake/TestFileOffsetBits.c
@@ -0,0 +1,11 @@
+#include <sys/types.h>
+
+/* Cause a compile-time error if off_t is smaller than 64 bits */
+#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
+int off_t_is_large[ (LARGE_OFF_T % 2147483629 == 721 && LARGE_OFF_T % 2147483647 == 1) ? 1 : -1 ];
+
+int main(int argc, char **argv)
+{
+  return 0;
+}
+
diff --git a/cmake/TestLargeFiles.c.cmake.in b/cmake/TestLargeFiles.c.cmake.in
@@ -0,0 +1,23 @@
+#cmakedefine _LARGEFILE_SOURCE
+#cmakedefine _LARGE_FILES
+#cmakedefine _FILE_OFFSET_BITS @_FILE_OFFSET_BITS@
+
+#include <sys/types.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+int main(int argc, char **argv)
+{
+  /* Cause a compile-time error if off_t is smaller than 64 bits,
+   * and make sure we have ftello / fseeko.
+   */
+#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
+  int off_t_is_large[ (LARGE_OFF_T % 2147483629 == 721 && LARGE_OFF_T % 2147483647 == 1) ? 1 : -1 ];
+  FILE *fp = fopen(argv[0],"r");
+  off_t offset = ftello( fp );
+
+  fseeko( fp, offset, SEEK_CUR );
+  fclose(fp);
+  return 0;
+}
+
diff --git a/cmake/TestWindowsFSeek.c b/cmake/TestWindowsFSeek.c
@@ -0,0 +1,10 @@
+#include <stdio.h>
+
+int main()
+{
+  __int64 off=0;
+
+  _fseeki64(NULL, off, SEEK_SET);
+
+  return 0;
+}
diff --git a/src/DiskFile.cpp b/src/DiskFile.cpp
@@ -82,11 +82,7 @@ CDiskFile::CDiskFile(CConfigurator *cfg, CSystem *sys, CDiskController *c,
     checkFileWritable(filename);
   }
 
-#ifdef HAVE_FOPEN64
-  handle = fopen64(filename, read_only ? "rb" : "rb+");
-#else
   handle = fopen(filename, read_only ? "rb" : "rb+");
-#endif
 
   // determine size...
   fseek_large(handle, 0, SEEK_END);

diff --git a/src/StdAfx.hpp b/src/StdAfx.hpp
@@ -163,15 +163,6 @@ inline char printable(char c) {
   return '.';
 }
 
-// Different OS'es define different functions to access 64-bit files
-#if defined(HAVE_FOPEN64)
-#define fopen_large fopen64
-#elif defined(HAVE_FOPEN)
-#define fopen_large fopen
-#else
-#error "Need fopen"
-#endif
-
 #if defined(HAVE__FSEEKI64)
 #define fseek_large _fseeki64
 #elif defined(HAVE_FSEEKO64)

diff --git a/src/config.hpp.in b/src/config.hpp.in
@@ -16,6 +16,21 @@
 /* Define to 1 if you have the <errno.h> header file. */
 #cmakedefine HAVE_ERRNO_H
 
+/* Define to 1 if you have large file support */
+#cmakedefine HAVE_LARGE_FILES
+
+/* Define to 1 if you have large file support */
+#cmakedefine _LARGEFILE_SOURCE
+
+/* Define to 1 if you have large file support */
+#cmakedefine _LARGE_FILES
+
+/* Define to 64 if you have large file support */
+#cmakedefine _FILE_OFFSET_BITS @_FILE_OFFSET_BITS@
+
+/* Define to 1 if you have large file support */
+#cmakedefine AXPBOX_HAVE_FSEEKO @AXPBOX_HAVE_FSEEKO@
+
 /* Define to 1 if you have the <fcntl.h> header file. */
 #cmakedefine HAVE_FCNTL_H