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

Use fseeko and ftello on BSDs #868

Merged
merged 1 commit into from
Sep 4, 2021

Conversation

liz-desartiges
Copy link
Contributor

fseek64 seems to be specific to glibc and doesn't work an other unix platforms.
(musl and ulibc support it for glibc compatibility)

@liz-desartiges
Copy link
Contributor Author

I would recommend using _FILE_OFFSET_BITS 64 macro instead of adding all those define, but it seams that you don't use large file for 32bits systems (on linux)

see : https://ftp.gnu.org/old-gnu/Manuals/glibc-2.2.3/html_node/libc_13.html for explanations

@liz-desartiges
Copy link
Contributor Author

resulting in those changes instead

diff --git a/tinyxml2.cpp b/tinyxml2.cpp
index d32b2b3..052ed28 100755
--- a/tinyxml2.cpp
+++ b/tinyxml2.cpp
@@ -103,14 +103,8 @@ distribution.
 #if defined(_WIN64)
 	#define TIXML_FSEEK _fseeki64
 	#define TIXML_FTELL _ftelli64
-#elif defined(__APPLE__) || defined(__FreeBSD__) || defined(__OpenBSD__) \
-	|| defined(__NetBSD__) || defined(__DragonFly__) || defined(__ANDROID__)
-	#define TIXML_FSEEK fseeko
-	#define TIXML_FTELL ftello
-#elif defined(__unix__) && defined(__x86_64__)
-	#define TIXML_FSEEK fseeko64
-	#define TIXML_FTELL ftello64
-#else
+#elif defined(__unix__)
+	#define _FILE_OFFSET_BITS 64
 	#define TIXML_FSEEK fseek
 	#define TIXML_FTELL ftell
 #endif

@liz-desartiges
Copy link
Contributor Author

This macro should only be selected if the system provides mechanisms for handling large files. On 64 bit systems this macro has no effect since the *64 functions are identical to the normal functions.
this makes me think it is unnecessary in the first place to use the *64 variant for x86_64 versions

@leethomason leethomason merged commit a977397 into leethomason:master Sep 4, 2021
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 this pull request may close these issues.

None yet

2 participants