I'm calling PHYSFS_stat() on a directory contained in the root, and with a single ZIP mounted and a "root directory" set to "data"

In function verifyPath(), the "prepend the root directory, if any" section, fname[] is being set to "dat/dir" rather than "data/dir". The issue seems to be an off-by-one, when prepending the root directory.

Old code:

if (h->root)
{
    const int isempty = (*fname == '\0');
    fname -= h->rootlen - 1;
    strcpy(fname, h->root);
    if (!isempty)
        fname[h->rootlen - 2] = '/';
    *_fname = fname;
} /* if */

Modified code:

if (h->root)
{
    const int isempty = (*fname == '\0');
    fname -= h->rootlen;
    strcpy(fname, h->root);
    if (!isempty)
        fname[h->rootlen - 1] = '/';
    *_fname = fname;
} /* if */

This seems to fix the problem, but I'm not sure if it could introduce problems elsewhere.

This bug report was migrated from our old Bugzilla tracker.

These attachments are available in the static archive:

• Patch to update the LZMA SDK and LZMA archiver (lzma938.diff, text/plain, 2015-01-10 09:39:41 -0500, 4514974 bytes)

Reported in version: all
Reported for operating system, platform: All, PC

Comments on the original bug report:

On 2015-01-10 09:39:41 -0500, Jan Hellwig wrote:

Created attachment 3508
Patch to update the LZMA SDK and LZMA archiver

Currently PhysicsFS uses an old version of the LZMA SDK. Since then the SDK has been updates with bugfixes, new features, etc. Also the license of the LZMA SDK was placed in the public domain (was LGPL before).
As the API has changed quite a bit over the years the PhysicsFS LZMA archiver also needed to be update to work with the new LZMA SDK API.

I attached a patch file that does update the LZMA SDK as well as the LZMA archiver to the latest version.
If you prefer to use the latest stable LZMA SDK (9.20, 4 years old), I do have a patch for that as well.

While porting physfs to be usable on Wii U homebrew, it was discovered this leaks a stack pointer out of scope.

We found this because PHYSFS_exists was failing, so the parameters of createDirHandle were logged:

00;41;58;987: mountPoint pêÀT¸T/
00;41;58;987: dirName fs:/vol/external01/save/game

We have patched this to make the minimum for smallAlloc be zero to avoid this issue. Hopefully this is a trivial fix.

See SDL_endian.h in current git master, for e.g.

This bug report was migrated from our old Bugzilla tracker.

These attachments are available in the static archive:

• Patch against tip (file_6398.txt, text/plain, 2015-07-30 20:34:32 -0400, 732 bytes)

Reported in version: all
Reported for operating system, platform: All, All

Comments on the original bug report:

On 2015-07-30 20:34:32 -0400, rettichschnidi wrote:

Created attachment 3544
Patch against tip

physfsrwops_write does not return what is specified in the documentation of SDL's read function in SDL_RWops.

On 2015-07-30 20:38:05 -0400, rettichschnidi wrote:

/read function/write function/

This bug report was migrated from our old Bugzilla tracker.

These attachments are available in the static archive:

• Return the number of read objects instead of bytes and 0/-1 (SDL2/SDL1) on error and eof. (fix-physfsrwops_read-behaviour.patch, text/plain, 2015-05-27 19:02:05 -0400, 933 bytes)
• Fixed patch (file_6385.txt, text/plain, 2015-07-27 16:34:33 -0400, 692 bytes)

Reported in version: all
Reported for operating system, platform: Linux, PC

Comments on the original bug report:

On 2015-05-27 19:02:05 -0400, rettichschnidi wrote:

Created attachment 3535
Return the number of read objects instead of bytes and 0/-1 (SDL2/SDL1) on error and eof.

physfsrwops_read does not return what is specified in the documentation of SDL's read function in SDL_RWops.

On 2015-07-27 16:34:33 -0400, rettichschnidi wrote:

Created attachment 3543
Fixed patch

On 2015-07-28 15:15:58 -0400, Ryan C. Gordon wrote:

This patch (plus some other fixes to make the code correct for SDL 2.0) are now here: https://hg.icculus.org/icculus/physfs/rev/61d312843192

Thanks!

--ryan.

Hello, I want to use PhysicsFS 3.2.0 (with SDL2) on Android, I using JNI C++ native for my Android application, and I can't initialize with argv[0], it just crashes with segmentation fault (SIGSEGV) error.

However, passing NULL as first argument to PHYSFS_init() is working fine, but with no base directory (PHYSFS_getBaseDir() returns /)

In debugging, I have found the issue, and it's in physfs_platform_android.c at method __PHYSFS_platformCalcBaseDir in line 57:

My part of code where I initing the PhysFS:

#include <SDL.h>
#include <physfs.h>
#include <android/log.h>

int SDL_main(int argc, char *argv[])
{
	__android_log_print(ANDROID_LOG_INFO, "hksoft", "Hello, Android!");
	__android_log_print(ANDROID_LOG_INFO, "hksoft", "ARGC is: %d, ARGV[0] is: %s", argc, argv[0]); // ARGC is: 1, ARGV[0] is: app_process

	// Init PhysFS
	if (PHYSFS_init(argv[0])) { // <-- Segmentation fault
		__android_log_print(ANDROID_LOG_INFO, "PHYSFS", "Init Successful!");
	} else {
		__android_log_print(ANDROID_LOG_FATAL, "PHYSFS", "Init Failure! Error: %s", PHYSFS_getLastError());
		return 1;
	}

	// Log some PhysFS stuff (if PhysFS was inited successfully)
	const char *baseDir = PHYSFS_getBaseDir(); // /
	const char *userDir = PHYSFS_getUserDir(); // /data/
	const char *prefDir = PHYSFS_getPrefDir("hat_kid", "SomethingGame"); // (null)
	const char *writeDir = PHYSFS_getWriteDir(); // (null)
	__android_log_print(ANDROID_LOG_INFO, "PHYSFS", "Base directory is: %s", baseDir); // Base directory is: /
	__android_log_print(ANDROID_LOG_INFO, "PHYSFS", "User directory is: %s", userDir); // User directory is: /data/
	__android_log_print(ANDROID_LOG_INFO, "PHYSFS", "Pref directory is: %s", prefDir); // Pref directory is: (null)
	__android_log_print(ANDROID_LOG_INFO, "PHYSFS", "Write directory is: %s", writeDir); // Write directory is: (null)

	// Init SDL2
	SDL_Init(SDL_INIT_VIDEO);

	...

	// Clean up
	SDL_Quit();

	return 0;
}

Screenshot of ADB logcat log:

Android version is 9, arch is armeabi-v7a (32-bit).

This bug report was migrated from our old Bugzilla tracker.

These attachments are available in the static archive:

• Use PHYSFS_sint64 instead of long (fix-physfsrwops_seek-signature.patch, text/plain, 2015-05-27 19:07:19 -0400, 773 bytes)

Reported in version: all
Reported for operating system, platform: Linux, PC

Comments on the original bug report:

On 2015-05-27 19:07:19 -0400, rettichschnidi wrote:

Created attachment 3536
Use PHYSFS_sint64 instead of long

The signature of physfsrwops_seek should take and return fixed size integers when used with SDL2.

On 2015-07-28 17:15:34 -0400, rettichschnidi wrote:

Fixed in https://hg.icculus.org/icculus/physfs/rev/61d312843192

This bug report was migrated from our old Bugzilla tracker.

These attachments are available in the static archive:

• Fix ISO9660FileFlags struct size on msvc (physfs-iso9660-fix-ISO9660FileFlags-size-in-msvc.diff, text/plain, 2015-04-10 12:34:56 -0400, 1163 bytes)

Reported in version: all
Reported for operating system, platform: Windows Vista, PC

Comments on the original bug report:

On 2015-04-10 12:34:56 -0400, Jonathan Hamilton wrote:

Created attachment 3532
Fix ISO9660FileFlags struct size on msvc

MSVC (at least version 13 - 'VS Express 2013 for Desktop') appear to expect the minimum size of packed bitfields to be specified. This means using 'unsigned' causes the size of the flags field to be 4 bytes (instead of the expected one).

This then causes the sizeof() the various record structs to be incorrect, causing them to be incorrectly read from the file.

Tested on windows 7 and windows 8.1 built with 'VS Express 2013 for Desktop'

Currently I'm having some issues with efficiently calling physfs from multiple threads. For some context here's what I've got going on:

• A task that does a massive enumeration through the entire virtual filesystem (to find archives to mount in the background)
• A task to enumerate and find the items to display in my tool's file browser
• A task that just changes what dirs are mounted when the user changes some path in my tool's settings

You can assume that all of this is running on different threads. The problem I ran into is that my massive enumeration will block the other two. Do you have any ideas for this issue? Here are the three solutions I've come up with, but they all have caveats:

1. Implement shared mutexes, where a unique lock is only used in functions that modify state. Cons: This only alleviates the issue for concurrent function calls that do not modify state, and would require changes to the native implementations.
2. Make a copy of needed state at the beginning of expensive functions, so the mutex only needs to be locked during cloning. Cons: cloning the opaque pointers, and cloning state would have to be carefully considered per each function it's implemented on so it doesn't cause unexpected outcomes.
3. Instantiating physfs contexts, as it appears you have planned for V4. Cons: the user has to manage the state of contexts across threads.

I didn't rule out option 1 because although it doesn't fully solve the problem on its own it could be a minor optimization to employ alongside another solution. Honestly it might make sense to do all three, as option two would help with performance for those who don't choose to maintain multiple contexts. All in all, 3 seems the most promising, so I'm glad that you have it planned.

Currently projects using CPM that use this library will have their RPATH broken for all targets defined after the CPMAddPackage for physfs, unless CMAKE_SKIP_RPATH is force-set to OFF.

Example:

CPMAddPackage(
  NAME physfs
  GIT_REPOSITORY https://github.com/icculus/physfs
  GIT_TAG release-3.2.0
  OPTIONS "CMAKE_INSTALL_PREFIX ${CMAKE_INSTALL_PREFIX}"
                  "PHYSFS_ARCHIVE_GRP OFF"
		  "PHYSFS_ARCHIVE_WAD OFF"
		  "PHYSFS_ARCHIVE_HOG OFF"
		  "PHYSFS_ARCHIVE_MVL OFF"
		  "PHYSFS_ARCHIVE_QPAK OFF"
		  "PHYSFS_ARCHIVE_SLB OFF"
		  "PHYSFS_ARCHIVE_ISO9660 OFF"
		  "PHYSFS_ARCHIVE_VDF OFF")

# Cleanup after physfs' use of set to avoid breaking the linker.
if(CMAKE_COMPILER_IS_GNUCC OR CMAKE_C_COMPILER_ID MATCHES "Clang")
  set(CMAKE_SKIP_RPATH OFF CACHE BOOL "Skip RPATH" FORCE)
endif()

Having an option to turn this behaviour off would be appreciated.

I can provide a small pull request that implements this, if this feature is accepted.

I contribute to the LÖVE game framework, which uses PhysFS, and a common user complaint has been that while the write-to-single-common-directory paradigm works OK for most games, it's too limited for things like tools and non-standard games or other apps that need to interact with the filesystem in more arbitrary manners.

Another related issue is that many operating systems (e.g. macOS, iOS, Windows) really want apps to use different common data locations for slightly different purposes. For example on iOS, files saved to Documents and Application Support may be automatically cloud-synced whereas files saved to tmp and Caches will not be. And Documents is meant for user-visible content (e.g. screenshots) whereas Application Support is not (e.g. a progress save file). Since iOS has restrictive sandboxing, I don't believe setting / as the write directory works.

In my opinion having to change the write directory - and having to make sure all open-for-write files are closed before doing so - each time a file is written to a different one of those locations is much too cumbersome and limiting, and doesn't fit with PhysFS' other virtual filesystem APIs.

To lift those limitations in LÖVE, I forked PhysFS and added a new PHYSFS_mountRW function which is just like PHYSFS_mount but with some internals changed to allow writing to the given location. Since write access tends to be allowed in fewer locations than read access, having a separate function from PHYSFS_mount still makes a lot of sense to me (although I suppose an enum or boolean parameter would work too, but it would also be a breaking change).

Here is my branch with the change: main...slime73:mountRW It's been tested but not by a ton of people yet, and I don't have a ton of experience with PhysFS' codebase so there might be bugs.

My questions are: is the overall idea within the scope of what you want PhysFS to be, and if so is my API reasonable? If yes I can open a pull request. I don't mind maintaining a fork but I'd prefer having the idea upstreamed.

This bug report was migrated from our old Bugzilla tracker.

These attachments are available in the static archive:

• Copied error dump from XCode (physfs-error.txt, text/plain, 2017-04-19 17:27:15 -0400, 1085 bytes)

Reported in version: all
Reported for operating system, platform: MacOS X, Macintosh

Comments on the original bug report:

On 2017-04-19 17:27:15 -0400, Tyler Funk wrote:

Created attachment 3579
Copied error dump from XCode

This issue is occurring when attempting to run L�VE 2D on the iOS simulator. I posted in their forums, and they were convinced it was a PhysicsFS issue, not a L�VE issue.

I am using the physfs.framework provided in the love-osx-frameworks-0.10.zip download located here: https://love2d.org/sdk/

If you have questions or need more information, please let me know. The crash does not provide much of a stack trace but I'll do what I can.

L�VE version: 0.10.1
XCode version: 8.3.2 (8E2002)
iOS Simulator version: 10.0
iOS Simulation: iPhone 6s - iOS 10.3 (14E269)

SDL2_image has an amazing test suite that checks loaders. While it would be great to have that for PhysFS (i.e. have a set of archives with identical content that are compared against reference directory), something simple like

#include <stdio.h>
#include <stdlib.h>

#define PHYSFS_DEPRECATED
#include "physfs.h"

int main(int argc, char **argv) {
    if (!PHYSFS_init(argv[0])) {
        printf("PHYSFS_init() failed!\n  reason: %s.\n", PHYSFS_getLastError());
        return 1;
    }

    printf("PhysFS initialized\n");

    if (!PHYSFS_deinit()) {
        printf("PHYSFS_deinit() failed!\n  reason: %s.\n", PHYSFS_getLastError());
        return 1;
    }

    printf("PhysFS deinitialized\n");
    return 0;
}

would still be fine to confirm that program using PhysFS links and loads.

If PHYSFS_setRoot() is called, the directory name isn't freed on shutdown.

In function "static int freeDirHandle(DirHandle *dh, FileHandle *openList)", add the line:

allocator.Free(dh->root);

The migration document for SDL3 outlines some changes to the SDL_RWops read and write function signatures here.

Naturally, this means the custom RWops implementation example in (extras/physfsrwops.c) will need an update, since it passes functions that use the old signature.

With SDL3 still in a prerelease state, I would imagine it would be currently inadvisable to push any changes until that's solidified. That said, I figured I'd make a note of it here so that it's not forgotten...and because I don't know what I'm looking at yet, I'm not sure how to fix it myself.

The last Release was some years ago again. ("March 18th, 2019: 3.0.2 released!")
Would probably make sense to make a Release soon.

I am looking into using PhysFS on an open-source game engine re-implementation and I was putting together a small test program that was supposed to be able to mount an ISO file (the original game data), a ZIP archive (a MOD) and a directory (user-provided resources), using the latest PhysFS release - 3.2.0 at the time of this writing.

Mounting the ZIP file and the directory is successful, however, mounting the ISO fails with error code 18: corrupted.

This is identical with the issue encountered by the OpenApoc project, and they "fixed" by patching PhysFS itself: JonnyH/physfs-hg-import@ef1c6ab

I've asked the developer that pushed the "fix" about fixing it upstream and he told me this:

I didn't really root-cause it, I suspect it's related to iso "formats" being a loose collection of slightly-incompatible extensions from different vendors - there's like 30 different ways of supporting longer filenames and different collections of file attributes.

If I apply that change to the PhysFS code the ISO seems to mount (and be parsed/files read) just fine.

Is that "fix" valid? Is there a way to make PhysFS ignore such errors already?

I have attached my test program, it includes my code, the PhysFS code, and the ISO, ZIP and data directories. Some notes are in the README.txt file found in the archive.

physfs-test.zip.

Like it is in subjest looks like cmake fails wuen cmake is executed with PHYSFS_BUILD_TEST=ON

+ /usr/bin/cmake -B x86_64-redhat-linux-gnu -D BUILD_SHARED_LIBS=ON -D CMAKE_AR=/usr/bin/gcc-ar -D CMAKE_BUILD_TYPE=RelWithDebInfo -D CMAKE_C_FLAGS_RELEASE=-DNDEBUG -D CMAKE_CXX_FLAGS_RELEASE=-DNDEBUG -D CMAKE_Fortran_FLAGS_RELEASE=-DNDEBUG -D CMAKE_INSTALL_PREFIX=/usr -D CMAKE_NM=/usr/bin/gcc-nm -D CMAKE_RANLIB=/usr/bin/gcc-ranlib -D CMAKE_VERBOSE_MAKEFILE=ON -D INCLUDE_INSTALL_DIR=/usr/include -D LIB_INSTALL_DIR=/usr/lib64 -D LIB_SUFFIX=64 -D SHARE_INSTALL_PREFIX=/usr/share -D SYSCONF_INSTALL_DIR=/etc -S . -D PHYSFS_BUILD_SHARED=ON -D PHYSFS_BUILD_STATIC=OFF -D PHYSFS_BUILD_TEST=ON
CMake Deprecation Warning at CMakeLists.txt:12 (cmake_minimum_required):
  Compatibility with CMake < 2.8.12 will be removed from a future version of
  CMake.

  Update the VERSION argument <min> value or use a ...<max> suffix to tell
  CMake that the project does not need compatibility with older versions.


-- The C compiler identification is GNU 12.0.1
-- The CXX compiler identification is GNU 12.0.1
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /usr/bin/gcc - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/g++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Found Doxygen: /usr/bin/doxygen (found version "1.9.4") found components: doxygen dot
-- PhysicsFS will build with the following options:
--   ZIP support: enabled
--   7zip support: enabled
--   GRP support: enabled
--   WAD support: enabled
--   HOG support: enabled
--   MVL support: enabled
--   QPAK support: enabled
--   SLB support: enabled
--   VDF support: enabled
--   ISO9660 support: enabled
--   Build static library: disabled
--   Build shared library: enabled
--   Build stdio test program: enabled
--     Use readline in test program: enabled
-- Configuring done
CMake Error: The following variables are used in this project, but they are set to NOTFOUND.
Please set them or make sure they are set and tested correctly in the CMake files:
CURSES_LIBRARY
    linked by target "test_physfs" in directory /home/tkloczko/rpmbuild/BUILD/physfs-3.0.2

-- Generating done
CMake Warning:
  Manually-specified variables were not used by the project:

    CMAKE_Fortran_FLAGS_RELEASE
    INCLUDE_INSTALL_DIR
    LIB_INSTALL_DIR
    SHARE_INSTALL_PREFIX
    SYSCONF_INSTALL_DIR


CMake Generate step failed.  Build files cannot be regenerated correctly.

BTW: do you have any plans to make new release?

Hello,

I'm trying PhyFS, seams really intresting but there is no example how to use write.
Result is a corrupted zip file
It would be great if I can get some help to understand what is wrong.
I'm using 3.3 release.
here is my simple code:
PHYSFS_init("d:\");
PHYSFS_setWriteDir("d:\");

PHYSFS_mount("d:\\testw.zip", "/" , true);       

const char* writedir =  PHYSFS_getWriteDir();
std::cout << writedir << std::endl; // for testing


PHYSFS_File* txtwtest = PHYSFS_openWrite("/txt2.txt");
int test = PHYSFS_writeBytes(txtwtest, "testwr", 5); // return 5 so it did write somewhere
PHYSFS_close(txtwtest);

PHYSFS_unmount("d:\\test.zip");
PHYSFS_deinit();

This bug report was migrated from our old Bugzilla tracker.

Reported in version: all
Reported for operating system, platform: Linux, PC

Comments on the original bug report:

On 2013-02-24 17:56:29 -0500, Julian Ospald (hasufell) wrote:

pkgconfig files (*.pc) are common interfaces that standardize compiler options for libraries across distros and are generated during build time

this helps build systems that rely on your library, so they don't need to write their own extensive checks, but just call pkg-config on your .pc file and get the correct includedir etc

http://www.freedesktop.org/wiki/Software/pkg-config

On 2014-04-27 18:18:56 -0400, rettichschnidi wrote:

I guess this is obsolete by now.

Commit: Added pkg-config support (thanks, Jonas!).
Link: https://hg.icculus.org/icculus/physfs/rev/aae614cdd005

PHYSFS_mkdir is called to create path in Application Support folder

Path passed is like "/var/mobile/Containers/Data/Application/A493CCAC-6FEE-434B-A5D4-1131850578C1/Library/Application Support/savegame".

I traced the problem to the doMkdir https://github.com/icculus/physfs/blob/main/src/physfs.c#L2237

It exits loop at first iteration because statbuf.filetype is PHYSFS_FILETYPE_SYMLINK for /var.

Not sure what the correct fix would be. Maybe to call stat with path ending with / like /var/ in this case statbuf.filetype is set correctly toPHYSFS_FILETYPE_DIRECTORY and rest works.

Also, (statbuf.filetype == PHYSFS_FILETYPE_DIRECTORY || statbuf.filetype == PHYSFS_FILETYPE_SYMLINK) can work around the problem in this case and the folder gets created at the end.

This bug report was migrated from our old Bugzilla tracker.

Reported in version: all
Reported for operating system, platform: All, PC

Comments on the original bug report:

On 2013-06-04 01:27:59 -0400, [email protected] wrote:

When compiling physicsfs with gcc on a 32bit Linux or OSX system, the library fails to link with the following error:

undefined reference to `__divdi3'

This is because of the 64 bit division in PHYSFS_read() and PHYSFS_write() with the line:

return ( (retval <= 0) ? retval : (retval / ((PHYSFS_sint64) size)) );

I found some similar discussion on the topic here:

http://stackoverflow.com/questions/35463/how-to-divide-two-64-bit-numbers-in-linux-kernel

Thanks for any help

On 2013-06-04 10:08:36 -0400, [email protected] wrote:

I forgot to add that this occurs with static linking

On 2013-12-05 15:50:23 -0500, Matthias Mailänder wrote:

I think http://software.opensuse.org/package/libphysfs1 are also effected by this. It does not work at all for me on i586 (tried Blobby Volley 2 and Hedgewars).

This is preliminary, but here's some initial wishlist things I'd like to do:

• Make the Android stuff not use PHYSFS_Init with a magic structure. This was a wrong move.
• Move all the global state into a struct and let people create instances, each with their own search path, write dir, mutexes, etc. All the functions will take an instance pointer; for people that don't care about this or want to build legacy code, we'll leave the original entry points alone, and those will manage an internal instance (or write a physfs3-compat library).
• Remove CD-ROM support (entry point will remain but never find any discs).
• Multiple write directories; they'll interpolate and mount and such like the search path does.
• Queries for other writeable directories (config/cache/etc), instead of just a "pref path"
• Probably other things.

This has been affecting VVVVVV, which uses PhysFS, and which uses a data.zip file with the game's assets (found here, via [Desktop data file]). Whenever people would add/remove any file in the zip with Windows Explorer, the entire zip would become (seemingly) completely unreadable to the game.

Digging into this deeper, this is caused by a validity check in PhysFS. These are some reproduction steps:

1. Start a zip file with 2 files with software that sets the Extract OS in file's headers to something other than 00 'MS-DOS' (I used Engrampa which came with my Linux distro, apparently zip from the terminal does not qualify.) Here's my version: test-2unix.zip

2. Observe the headers with zipdetails:

0000 LOCAL HEADER #1       04034B50
0004 Extract Zip Spec      0A '1.0'
0005 Extract OS            03 'Unix'
[...]

0037 LOCAL HEADER #2       04034B50
003B Extract Zip Spec      0A '1.0'
003C Extract OS            03 'Unix'
[...]

006E CENTRAL HEADER #1     02014B50
0072 Created Zip Spec      3F '6.3'
0073 Created OS            03 'Unix'
0074 Extract Zip Spec      0A '1.0'
0075 Extract OS            03 'Unix'
[...]

00C5 CENTRAL HEADER #2     02014B50
00C9 Created Zip Spec      3F '6.3'
00CA Created OS            03 'Unix'
00CB Extract Zip Spec      0A '1.0'
00CC Extract OS            03 'Unix'
[...]

3. In Windows Explorer (tested with Windows 10 and 11), drag and drop a third file into the zip. My result: test-2unix-1ms.zip

4. Observe the new headers with zipdetails:

0000 LOCAL HEADER #1       04034B50
0004 Extract Zip Spec      0A '1.0'
0005 Extract OS            03 'Unix'
[...]

0037 LOCAL HEADER #2       04034B50
003B Extract Zip Spec      0A '1.0'
003C Extract OS            03 'Unix'
[...]

006E LOCAL HEADER #3       04034B50
0072 Extract Zip Spec      14 '2.0'
0073 Extract OS            00 'MS-DOS'
[...]

00A5 CENTRAL HEADER #1     02014B50
00A9 Created Zip Spec      14 '2.0'
00AA Created OS            00 'MS-DOS'
00AB Extract Zip Spec      14 '2.0'
00AC Extract OS            00 'MS-DOS'
[...]

00FC CENTRAL HEADER #2     02014B50
0100 Created Zip Spec      14 '2.0'
0101 Created OS            00 'MS-DOS'
0102 Extract Zip Spec      14 '2.0'
0103 Extract OS            00 'MS-DOS'
[...]

0153 CENTRAL HEADER #3     02014B50
0157 Created Zip Spec      14 '2.0'
0158 Created OS            00 'MS-DOS'
0159 Extract Zip Spec      14 '2.0'
015A Extract OS            00 'MS-DOS'
[...]

5. PhysFS now considers the first two files corrupted and refuses to process them.

So what Windows Explorer apparently does when it modifies the zip file is update the version numbers/OS for all existing files in the central headers, but it leaves them as-is in the local headers.

The check in PhysFS which fails is in zip_parse_local in physfs_archiver_zip.c, specifically ui16 != entry->version_needed. ui16 being Extract Zip Spec/Extract OS in a local header, and entry->version_needed being Extract Zip Spec/Extract OS in the corresponding central header.

Adding this printf demonstrates it:

diff --git a/src/physfs_archiver_zip.c b/src/physfs_archiver_zip.c
index 9972628..99d4dbc 100644
--- a/src/physfs_archiver_zip.c
+++ b/src/physfs_archiver_zip.c
@@ -833,6 +833,9 @@ static int zip_parse_local(PHYSFS_Io *io, ZIPentry *entry)
     BAIL_IF_ERRPASS(!readui32(io, &ui32), 0);
     BAIL_IF(ui32 != ZIP_LOCAL_FILE_SIG, PHYSFS_ERR_CORRUPT, 0);
     BAIL_IF_ERRPASS(!readui16(io, &ui16), 0);
+    printf("ui16: 0x%04x, entry->version_needed: 0x%04x\n",
+        ui16, entry->version_needed
+    );
     BAIL_IF(ui16 != entry->version_needed, PHYSFS_ERR_CORRUPT, 0);
     BAIL_IF_ERRPASS(!readui16(io, &ui16), 0);  /* general bits. */
     BAIL_IF_ERRPASS(!readui16(io, &ui16), 0);

The output from my "minimal" test program (main.c) is as follows for the Unix-only zip:

$ ./main test-2unix.zip 
ui16: 0x030a, entry->version_needed: 0x030a
ui16: 0x030a, entry->version_needed: 0x030a
File a.txt: 20/20 bytes
File b.txt: 20/20 bytes

And this for the Explorer-modified zip:

$ ./main test-2unix-1ms.zip 
ui16: 0x0014, entry->version_needed: 0x0014
ui16: 0x030a, entry->version_needed: 0x0014
Cannot enumerate files! corrupted

So, I don't know whether Explorer is in the wrong here by updating one type of header and not the other, or whether PhysFS makes an incorrect assumption that this is invalid, but other software seems to have no problems with these zips.

My use case is an archiver that reads the entire container file into memory in openArchive and serves chunks of memory in openRead by using createMemoryIo; attached test is reduced (so appears nonsensical)

The core problem are the different semantics of the ATOMIC_DECR impls https://github.com/icculus/physfs/blob/main/src/physfs_internal.h#L116 ff:

• _InterlockedDecrement : https://docs.microsoft.com/en-us/windows/win32/api/winnt/nf-winnt-interlockeddecrement

The function returns the resulting decremented value.

• __sync_fetch_and_add : https://gcc.gnu.org/onlinedocs/gcc/_005f_005fsync-Builtins.html

and returns the value that had previously been in memory

In https://github.com/icculus/physfs/blob/main/src/physfs.c#L356

    assert(info->refcount > 0);  /* even in a race, we hold a reference. */

    if (__PHYSFS_ATOMIC_DECR(&info->refcount) == 0)

If refcount > 0 and the impl is __sync_fetch_and_add(ptrval, -1) the return value will still be > 0 and the cleanup block is never used.

The fallback impl also returns the unmodified value; haven't looked at the watcom asm.
note: ATOMIC_INCR has the same problem but the return value is never used.

valgrind --leak-check=full --show-reachable=yes ./a.out
…
==862567== 6 bytes in 1 blocks are indirectly lost in loss record 1 of 3
==862567==    at 0x483877F: malloc (vg_replace_malloc.c:307)
==862567==    by 0x1130EF: mallocAllocatorMalloc (physfs.c:3248)
==862567==    by 0x10C531: ABC_openRead (test_memio.c:36)
==862567==    by 0x112112: PHYSFS_openRead (physfs.c:2767)
==862567==    by 0x10C6DA: main (test_memio.c:111)
==862567== 
==862567== 48 bytes in 1 blocks are indirectly lost in loss record 2 of 3
==862567==    at 0x483877F: malloc (vg_replace_malloc.c:307)
==862567==    by 0x1130EF: mallocAllocatorMalloc (physfs.c:3248)
==862567==    by 0x10D051: __PHYSFS_createMemoryIo (physfs.c:392)
==862567==    by 0x10C560: ABC_openRead (test_memio.c:38)
==862567==    by 0x112112: PHYSFS_openRead (physfs.c:2767)
==862567==    by 0x10C6DA: main (test_memio.c:111)
==862567== 
==862567== 134 (80 direct, 54 indirect) bytes in 1 blocks are definitely lost in loss record 3 of 3
==862567==    at 0x483877F: malloc (vg_replace_malloc.c:307)
==862567==    by 0x1130EF: mallocAllocatorMalloc (physfs.c:3248)
==862567==    by 0x10D029: __PHYSFS_createMemoryIo (physfs.c:390)
==862567==    by 0x10C560: ABC_openRead (test_memio.c:38)
==862567==    by 0x112112: PHYSFS_openRead (physfs.c:2767)
==862567==    by 0x10C6DA: main (test_memio.c:111)

Those leaks go away when replacing __sync_fetch_and_add with __sync_add_and_fetch.
test_memio.c.txt

I'm trying to write an archiver with streaming write support (a simple format covered by the unpacked template). I implemented an openWrite() function I wanted to test, but PHYSFS_openWrite() never reaches it. It fails in verifyPath() because it uses the directory archiver's stat function, determines it doesn't exist, and returns an error.

It calls DIR_stat() here, and here's an example of my source code:

const char packname[] = "Armor_012_Upper.pack";

int main(int argc, char** argv) {
    PHYSFS_init(argv[0]);
    PHYSFS_mount(PHYSFS_getBaseDir(), NULL, true);
    PHYSFS_setWriteDir(PHYSFS_getBaseDir());

    PHYSFS_registerArchiver(&archiver_sarc_default);

    printf("Mounting %s...\n\n", packname);
    PHYSFS_mount(packname, NULL, true);

    PHYSFS_file* test_write = PHYSFS_openWrite("/ActorLink/Armor_012_Upper.bxml");
    printf("%p\n", test_write);
}

I'm able to read the file into a buffer with PHYSFS_openRead() and PHYSFS_readBytes() and write it to disk (I omitted the code for this to keep the code snippet short). So the file definitely exists, and the archiver is registered correctly.

TLDR: Writing has very different behaviour from reading which seems to prevent me from even reaching the writing code in my archiver. I could try to re-implement PHYSFS_openWrite() myself, but I would prefer not to if possible.

Hello, I would like to clarify whether it is possible to create your own archive format using the library?

If so, could you briefly explain what procedures need to be performed?

From CI run logs:

In file included from /home/runner/work/physfs/physfs/src/physfs.c:12:
/home/runner/work/physfs/physfs/src/physfs.c: In function ‘openDirectory’:
/home/runner/work/physfs/physfs/src/physfs.c:929:40: warning: ?: using integer constants in boolean context [-Wint-in-bool-context]
  929 |     BAIL_IF(!retval, claimed ? errcode : PHYSFS_ERR_UNSUPPORTED, NULL);
      |                      ~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~
/home/runner/work/physfs/physfs/src/physfs_internal.h:273:44: note: in definition of macro ‘BAIL_IF’
  273 | #define BAIL_IF(c, e, r) do { if (c) { if (e) PHYSFS_setErrorCode(e); return r; } } while (0)
      |                                            ^

Is the following correct??

diff --git a/src/physfs.c b/src/physfs.c
index e7ceddd..568f1fc 100644
--- a/src/physfs.c
+++ b/src/physfs.c
@@ -921,12 +921,12 @@ static DirHandle *openDirectory(PHYSFS_Io *io, const char *d, int forWriting)
             retval = tryOpenDir(io, *i, d, forWriting, &claimed);
     } /* else */
 
-    errcode = currentErrorCode();
+    errcode = claimed ? currentErrorCode() : PHYSFS_ERR_UNSUPPORTED;
 
     if ((!retval) && (created_io))
         io->destroy(io);
 
-    BAIL_IF(!retval, claimed ? errcode : PHYSFS_ERR_UNSUPPORTED, NULL);
+    BAIL_IF(!retval, errcode, NULL);
     return retval;
 } /* openDirectory */
 

This bug report was migrated from our old Bugzilla tracker.

These attachments are available in the static archive:

• Implement physfsrwops_size for for SDL_RWops' size callback. (implement-SDL_RWsize-on-SDL2.patch, text/plain, 2015-05-27 19:12:19 -0400, 1296 bytes)

Reported in version: all
Reported for operating system, platform: Linux, PC

Comments on the original bug report:

On 2015-05-27 19:12:19 -0400, rettichschnidi wrote:

Created attachment 3537
Implement physfsrwops_size for for SDL_RWops' size callback.

SDL2 comes with the macro SDL_RWsize. Therefore, PhysicsFS should support the size callback of SDL_RWops.

On 2015-09-15 14:33:51 -0400, rettichschnidi wrote:

Issue got already fixed: http://hg.icculus.org/icculus/physfs/rev/61d312843192#l2.136

This bug report was migrated from our old Bugzilla tracker.

These attachments are available in the static archive:

• Simple Android.mk file. Enables ZIP and APK support. (Android.mk, text/x-makefile, 2014-04-27 18:48:46 -0400, 585 bytes)
• Patch with the implementation and the required changes for src/physfs.c and the CMake build script (physfs_apk_support.patch, text/plain, 2014-04-27 18:50:33 -0400, 4951 bytes)

Reported in version: all
Reported for operating system, platform: other, Other

Comments on the original bug report:

On 2014-04-27 18:48:46 -0400, rettichschnidi wrote:

Created attachment 3439
Simple Android.mk file. Enables ZIP and APK support.

Based on the archiver_zip.c file I created a super simple hack which allows to use physfs to mount an Android APK file.

It basicly just prexfixes each filename with "/assets/" and then forwards the real work to the ZIP implementation in archiver_zip.c.

There is also a simple Android.mk file which is configured to enable ZIP and APK support.

On 2014-04-27 18:50:33 -0400, rettichschnidi wrote:

Created attachment 3440
Patch with the implementation and the required changes for src/physfs.c and the CMake build script

This bug report was migrated from our old Bugzilla tracker.

Reported in version: all
Reported for operating system, platform: MacOS X, Macintosh

Comments on the original bug report:

On 2018-09-02 01:32:34 -0400, wrote:

Hi,

In CMake I click Configure then Generate, all appears well. If I configured using Xcode, Xcode fails building with this error:

PhaseScriptExecution CMake\ PostBuild\ Rules build/PhysicsFS.build/Debug/physfs.build/Script-F7F66E2812A349EFB43CD3E4.sh
    cd /Users/christaylor/Programming/physfs-3.0.1
    /bin/sh -c /Users/christaylor/Programming/physfs-3.0.1/build/PhysicsFS.build/Debug/physfs.build/Script-F7F66E2812A349EFB43CD3E4.sh

echo "Creating symlinks"
Creating symlinks
/Applications/3rdParty/Programming/CMake.app/Contents/bin/cmake -E cmake_symlink_library /Users/christaylor/Programming/physfs-3.0.1/build/Debug/libphysfs.3.0.1.dylib /Users/christaylor/Programming/physfs-3.0.1/build/Debug/libphysfs.1.dylib /Users/christaylor/Programming/physfs-3.0.1/build/Debug/libphysfs.dylib
make: /Applications/3rdParty/Programming/CMake.app/Contents/bin/cmake: No such file or directory
make: *** [physfs_buildpart_0] Error 1

If I build the makefiles and run make, I get a very similar error:

make: /Applications/3rdParty/Programming/CMake.app/Contents/bin/cmake: No such file or directory
make: *** [cmake_check_build_system] Error 1

I am using Cmake 3.12.1 and Xcode 9.4.1.

just a minor inconvenience, but when opting to just drop physfs into our project (as advised in CMakeLists.txt), it's inconvenient to just add -Ipath/to/physfs/, because we would then we would have to include #include <src/physfs.h>. If we use -Ipath/to/physfs/src/ we would then include #include <physfs.h>.
Because I prefer prefixed headers, I would like to include like this <physfs/physfs.h>, but there's no way without renaming or symlinking or creating a new file.

We also don't have an include/, so one less reason for the src/ folder.

The CMake build currently uses include_directories() to add the src directory to the includes. It would be better to use target_include_directories() to have the target include the actual directory rather than in the global cmake space.

https://cmake.org/cmake/help/latest/command/target_include_directories.html

option(PHYSFS_BUILD_STATIC "Build static library" TRUE)
if(PHYSFS_BUILD_STATIC)
    add_library(physfs-static STATIC ${PHYSFS_SRCS})
    target_include_directories(physfs-static PUBLIC src)

Hi!

A very discrete problem:
i have a zip of an dos game. This is mounted via dosboxx via physfs overlay.
In this archive there is an file "sndcfg.ini". I need to replace this file while using the overlay. This is not possible. This the old file from the archive is used, although there is the new sndcfg.ini in the overlay part.

wish: in doubt use the overlays file (when its newer etc)

marco

I have this code, which seems like everything I need to do to create a file in the root dir. But it fails because the file handler is NULL. Could you tell me what is wrong with this?

  PHYSFS_init(argv[0]);
  if (not PHYSFS_mount(PHYSFS_getPrefDir("bkeys_games", "Call of the Dead"),
                       NULL, 1)) {
    std::cout << "WHY????" << std::endl;
  }
  if (PHYSFS_setWriteDir("/")) {
    PHYSFS_ErrorCode error = PHYSFS_getLastErrorCode();
    std::cout << "files are open?" << std::endl;
  }
  PHYSFS_Stat stat = PHYSFS_Stat();
  PHYSFS_File *txtwtest = PHYSFS_openWrite("settings.xml");
  PHYSFS_ErrorCode error = PHYSFS_getLastErrorCode();
  int test = PHYSFS_writeBytes(txtwtest, "I am bkeys, kneel before me",
                               strlen("I am bkeys, kneel before me"));
  PHYSFS_close(txtwtest);

I am trying to use PhysicsFS with SDL2, but I did not see an option to compile physfsrwops.c with cmake. Do I have to do it myself, or am I just missing something?

Is there a plan on adding support for watching a directory for changes? Ideally via native file watch API.

can you create a conan package manager recipe you this?

This bug report was migrated from our old Bugzilla tracker.

Reported in version: all
Reported for operating system, platform: Windows NT, PC

Comments on the original bug report:

On 2015-02-07 07:44:17 -0500, Aleksi Juvani wrote:

2.0.3 build fails on 64-bit Windows 8 with MinGW and GCC 4.8.0 with the following errors:

 physfs-2.0.3/archivers/lzma.c: In function 'SzFileReadImp':
 physfs-2.0.3/archivers/lzma.c:133:46: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast]
     FileInputStream *s = (FileInputStream *)((unsigned long)object - offsetof(FileInputStream, inStream)); /* HACK! */
                                              ^
physfs-2.0.3/archivers/lzma.c: In function 'SzFileSeekImp':
physfs-2.0.3/archivers/lzma.c:148:46: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast]
     FileInputStream *s = (FileInputStream *)((unsigned long)object - offsetof(FileInputStream, inStream)); /* HACK! */
                                              ^
cc1.exe: all warnings being treated as errors

The development branch from the repository fails to build with the following errors:

physfs/src/platform_windows.c: In function '__PHYSFS_platformEnumerateFiles':
physfs/src/platform_windows.c:533:21: error: unused variable 'tag' [Werror=unused-variable]
         const DWORD tag = entw.dwReserved0;
                 ^
physfs/src/platform_windows.c:532:21: error: unused variable 'attr' [Werror=unused-variable]
         const DWORD attr = entw.dwFileAttributes;
                 ^
physfs/src/platform_windows.c: At top level:
physfs/src/platform_windows.c:483:12: error: 'isSymlinkAttrs' defined but not used [-Werror=unused-function]
 static int isSymlinkAttrs(const DWORD attr, const DWORD tag)
        ^
cc1.exe: all warnings being treated as errors

Both build fine if I manually remove the -Werror flag from CMakeLists.txt. I'd recommend removing -Werror by default. It makes sense for development, but not as a default for every build.

On 2015-05-03 01:30:40 -0400, Bradley Bell wrote:

cast to unsigned long seems pretty unnecessary there.

diff -r aaa1204a4426 archivers/lzma.c
--- a/archivers/lzma.c  Thu Aug 14 21:27:00 2014 -0400
+++ b/archivers/lzma.c  Sat May 02 22:20:44 2015 -0700
@@ -130,7 +130,7 @@
 SZ_RESULT SzFileReadImp(void *object, void *buffer, size_t size,
                         size_t *processedSize)
 {
-    FileInputStream *s = (FileInputStream *)((unsigned long)object - offsetof(FileInputStream, inStream)); /* HACK! */
+    FileInputStream *s = (FileInputStream *)(object - offsetof(FileInputStream, inStream)); /* HACK! */
     size_t processedSizeLoc = __PHYSFS_platformRead(s->file, buffer, 1, size);
     if (processedSize != 0)
         *processedSize = processedSizeLoc;
@@ -145,7 +145,7 @@
  */
 SZ_RESULT SzFileSeekImp(void *object, CFileSize pos)
 {
-    FileInputStream *s = (FileInputStream *)((unsigned long)object - offsetof(FileInputStream, inStream)); /* HACK! */
+    FileInputStream *s = (FileInputStream *)(object - offsetof(FileInputStream, inStream)); /* HACK! */
     if (__PHYSFS_platformSeek(s->file, (PHYSFS_uint64) pos))
         return SZ_OK;
     return SZE_FAIL;

Hi,

Had a strange issue that has occurred regarding a zip created through an external library that mounts without error in physfs but any attempt to enumerate or stat on the files inside returns the 'not found' error.

I've tested in the integrity of the zip (through Ubuntu) and there are no issues (plus the files can be extracted in other tools without error), the only guess I can make on the problem is that the entries inside the zip are not separated as two parts directory and then the file (also probably perhaps something related to the leading slash in the filenames?).

E.g. (the problem zip shows this for the integrity check)

Zipping the files directly in Ubuntu gives a working file but looks like this...

Here are the two files and some example code to hopefully show the error...

not-working.zip
working.zip

int main( int argc, char* argv[] )
{
    if ( PHYSFS_init( NULL ) )
    {
        // This zip mounts fine (created by Ubuntu compress)
        if ( PHYSFS_mount( "working.zip", "root-good", 0 ) != 0 )
        {
            PHYSFS_Stat fileInfo0;
            if ( PHYSFS_stat( "root-good/0/test_file_0.txt", &fileInfo0 ) == 0 )
            {
                PHYSFS_ErrorCode errorCode = PHYSFS_getLastErrorCode();
                std::cout << PHYSFS_getErrorByCode( errorCode ) << std::endl;
            }
            else
            {
                std::cout << fileInfo0.filesize << std::endl;
            }

            PHYSFS_Stat fileInfo1;
            if ( PHYSFS_stat( "root-good/1/test_file_1.txt", &fileInfo1 ) == 0 )
            {
                PHYSFS_ErrorCode errorCode = PHYSFS_getLastErrorCode();
                std::cout << PHYSFS_getErrorByCode( errorCode ) << std::endl;
            }
            else
            {
                std::cout << fileInfo1.filesize << std::endl;
            }

            PHYSFS_unmount( "root-good" );
        }

        // This zip mounts but cannot find the files
        if ( PHYSFS_mount( "not-working.zip", "root-bad", 0 ) != 0 )
        {
            PHYSFS_Stat fileInfo0;
            if ( PHYSFS_stat( "root-bad/0/test_file_0.txt", &fileInfo0 ) == 0 )
            {
                PHYSFS_ErrorCode errorCode = PHYSFS_getLastErrorCode();
                std::cout << PHYSFS_getErrorByCode( errorCode ) << std::endl;
            }
            else
            {
                std::cout << fileInfo0.filesize << std::endl;
            }

            PHYSFS_Stat fileInfo1;
            if ( PHYSFS_stat( "root-bad/1/test_file_1.txt", &fileInfo1 ) == 0 )
            {
                PHYSFS_ErrorCode errorCode = PHYSFS_getLastErrorCode();
                std::cout << PHYSFS_getErrorByCode( errorCode ) << std::endl;
            }
            else
            {
                std::cout << fileInfo1.filesize << std::endl;
            }

            PHYSFS_unmount( "root-bad" );
        }
        PHYSFS_deinit();
    }
}

Thanks in advance for any enlightenment on this :)

Hi @icculus ,
since some time your website https://icculus.org/physfs/ refers to an outdated version of physfs and its api documentation: https://icculus.org/physfs/docs/html/ . Maybe you should update both.

Unity builds glue together multiple files, so when using PhysFS in a bigger project, macros like

#define malloc(x) Do not use malloc() directly.

(physfs_internal.h:171)
suddenly may become defined for an outer scope, effectively disabling malloc usage in this case.

Hi there,

my physfs-static lib build fails on Ubuntu in Github Actions, works on windows tho. It works on my Arch Linux desktop and Ubuntu desktop without problems, its only github Actions that fails. What could I be missing, don't know the whole idea behind the lib yet.

https://github.com/ntropy83/tinyGameEngine/actions/runs/6870720894

The Cmake configuration for my physfs implementation is in the folder engine.

Thanks for your help in advance !
Greetings ent

This bug report was migrated from our old Bugzilla tracker.

Reported in version: all
Reported for operating system, platform: Linux, PC

Comments on the original bug report:

On 2013-09-14 19:25:40 -0400, Reto Schneider wrote:

When building the current version (3b2e649c044c) of the PhysicsFS library in release mode, the variable "rc"[1] in physfs.c:1271 is unused and the compile process stops due to the "-Wall" compile mode.

Solution: (void)rc;

1: https://hg.icculus.org/icculus/physfs/file/3b2e649c044c/src/physfs.c#l1271

On 2015-07-28 17:13:18 -0400, rettichschnidi wrote:

This issue is no longer relevant: https://hg.icculus.org/icculus/physfs/rev/a3dabf75a0d0

In porting my application from PhysicsFS 2 to 3, I've run into the deprecation of PHYSFS_getUserDir. The suggested replacement, PHYSFS_getPrefDir, is not always the appropriate one. Of course, it could be that I'm doing stuff wrong.

Missing distinction between configuration and data

First of all, my application distinguishes between configuration and (writable) data. On Linux, it uses the following locations:

Configuration: PHYSFS_getUserDir() + "/.config/org/app"
Data: PHYSFS_getUserDir() + "/.local/share/app"

Only for the latter can I use PHYSFS_getPrefDir, though it's not the directory I had expected for a function with "Pref" in its name.

The situation is similar for Haiku, which distinguishes between configuration at /config/settings and data at /config/data (but PhysicsFS uses /config/settings here).

And there is Windows, where PHYSFS_getPrefDir is based on the folder CSIDL_APPDATA, but this is the roaming variant, whereas in our app we'd want CSIDL_LOCAL_APPDATA for the data.

Since in fact PHYSFS_getUserDir can only provide part of the functionality here, the solution might be to add a PHYSFS_getLocalDataDir, which would return a suitable path for storing local data, as opposed to user preferences.

Other user-cases

• We used to store configuration and data in ~/.app, and we still support migrating old config from there. PHYSFS_getUserDir was useful for this purpose.

• Our app supports saving of screenshots, which we currently put in PHYSFS_getUserDir() + "Desktop" by default. This is obviously a case of "you're doing it wrong" because this path is subject to localization and might not make sense at all, of course. On Windows we try to use CSIDL_MYPICTURES and CSIDL_DESKTOP here, but I'm not sure it makes sense for PhysicsFS to abstract all these locations.

Either way, I think to be able to get the base user directory in a platform-agnostic way remains useful.

PhysFS has some memory safe issues resulting in out-of-bound reads.
These issues were noticed by running tests using Clang's sanitizers (address,signed-integer-overflow).

An obvious impact is the ability to make software crash (ie Denial of Service) by providing malformed archives.
I have not investigated whether more serious exploits are possible or not. My focus is on reporting problems and recommending fixes.

• physfs_archiver_csm.c: Off-by-one error in array index may lead to accessing memory after the end of the name buffer.
• physfs_archived_qpak.c: Failure to validate the name is null-terminated may lead to accessing memory after the end of the name buffer.
• physfs_unicode.c: Off-by-one error in buffer increment may lead to accessing memory after the end of an unicode buffer.

An upcoming merge request will provide a possible fix and will points to specific lines of code.

When compiling SDL3 for tvOS using CMake, the configure script links to IOKit, causing this link error because IOKit is not present on tvOS:

ld: framework 'IOKit' not found
clang: error: linker command failed with exit code 1 (use -v to see invocation)

IOKit is not available when building for tvOS.

I made this quick-and-dirty patch in my own copy:

if(APPLE)
    if (NOT (CMAKE_SYSTEM_NAME MATCHES "tvOS"))
        set(iokit_lib "-framework IOKit")
    endif()
    set(OTHER_LDFLAGS ${OTHER_LDFLAGS} ${iokit_lib} "-framework Foundation")
    list(APPEND PHYSFS_M_SRCS src/physfs_platform_apple.m)
endif()

Last year Paradox developer Mathieu Ropert published a blog entry pointing to some potential enhancements in PhysFS regarding multithreading ( https://mropert.github.io/2020/07/26/threading_with_physfs/ ).

We are unaffiliated with Paradox or any Paradox products, but we encounter comparable performance problems in one of our projects when loading resources through PhysicsFS. Since modern APIs like Vulcan support multithreaded model-loading and since many machines now feature four, eight or even more cores and with ever-increasing SSD speeds, it is unfortunate that when using PhysicsFS, loading can effectively only be done on one thread at a time.

Is there any chance to see this bottleneck in PhysicsFS addressed in the future?

This bug report was migrated from our old Bugzilla tracker.

These attachments are available in the static archive:

• Fix read error checking (physfs-iso9660-fix-read-error-checking.diff, text/plain, 2015-04-10 12:28:41 -0400, 1293 bytes)
• Fix magic memcmp length (physfs-iso9660-compare-5-bytes-of-magic.diff, text/plain, 2015-04-10 12:29:13 -0400, 841 bytes)

Reported in version: all
Reported for operating system, platform: Linux, PC

Comments on the original bug report:

On 2015-04-10 12:28:41 -0400, Jonathan Hamilton wrote:

Created attachment 3530
Fix read error checking

As far as I can see, iso_readimage() and io->read() return the number of bytes read, but this seems wrong in a couple of places:

• Reading 5 bytes of the magic number appears to expect io->read() to return 'Not 5'. This value is then checked with a memcmp of 6 bytes, comparing 1-over the array.
• Reading the file descriptor appears to expect iso_readimage to return '1' on success, however it actually returns the number of bytes read.

Patches should be attached.

Thanks,
Jonathan

On 2015-04-10 12:29:13 -0400, Jonathan Hamilton wrote:

Created attachment 3531
Fix magic memcmp length