physfs icon indicating copy to clipboard operation
physfs copied to clipboard

Contexts

Open S41L0R opened this issue 2 years ago • 2 comments

This enables the use of PHYSFS with multiple different states at once, as discussed in #63.

S41L0R avatar Jul 26 '23 18:07 S41L0R

Fixes C89 build failures in CI:

diff --git a/src/physfs.c b/src/physfs.c
index 998157c..958b784 100644
--- a/src/physfs.c
+++ b/src/physfs.c
@@ -3498,10 +3498,11 @@ static int doDeinitContext(Context *context) {
 int PHYSFS_initContext(PHYSFS_Context _context, const char *argv0)
 {
     Context *context = (Context*)_context;
+    PHYSFS_Context boundContext;
 
     BAIL_IF(context->initialized, PHYSFS_ERR_IS_INITIALIZED, 0);
 
-    PHYSFS_Context boundContext = PHYSFS_getBoundContext();
+    boundContext = PHYSFS_getBoundContext();
     PHYSFS_bindContext(_context);
 
     if (!initializeContextMutexes(context)) goto initFailed;
@@ -3554,15 +3555,19 @@ int PHYSFS_deinitContext(PHYSFS_Context _context)
 
 int PHYSFS_bindContext(PHYSFS_Context context)
 {
+    ThreadContextMapNode *newNode;
+    ThreadContextMapNode *currentNode;
+    ThreadContextMapNode *lastNode;
+    void *threadID;
+
     __PHYSFS_platformGrabMutex(threadContextMapLock);
 
-    void *threadID = __PHYSFS_platformGetThreadID();
-    
+    threadID = __PHYSFS_platformGetThreadID();
     if (!context)
         context = defaultContext;
 
-    ThreadContextMapNode *currentNode = threadContextMap;
-    ThreadContextMapNode *lastNode = threadContextMap;
+    currentNode = threadContextMap;
+    lastNode = threadContextMap;
     if (currentNode)
     {
         do
@@ -3579,7 +3584,7 @@ int PHYSFS_bindContext(PHYSFS_Context context)
     } /* if */
     else
     {
-        ThreadContextMapNode *newNode = allocator.Malloc(sizeof(ThreadContextMapNode));
+        newNode = allocator.Malloc(sizeof(ThreadContextMapNode));
         GOTO_IF(!newNode, PHYSFS_ERR_OUT_OF_MEMORY, bindFailed);
         newNode->context = context;
         newNode->threadID = threadID;
@@ -3588,7 +3593,7 @@ int PHYSFS_bindContext(PHYSFS_Context context)
         goto bindSuccess;
     } /* else */
 
-    ThreadContextMapNode *newNode = allocator.Malloc(sizeof(ThreadContextMapNode));
+    newNode = allocator.Malloc(sizeof(ThreadContextMapNode));
     GOTO_IF(!newNode, PHYSFS_ERR_OUT_OF_MEMORY, bindFailed);
     newNode->context = context;
     newNode->threadID = threadID;
@@ -3607,11 +3612,14 @@ bindFailed:
 
 PHYSFS_Context PHYSFS_getBoundContext()
 {
+    ThreadContextMapNode *currentNode;
+    void *threadID;
+
     __PHYSFS_platformGrabMutex(threadContextMapLock);
 
-    void *threadID = __PHYSFS_platformGetThreadID();
+    threadID = __PHYSFS_platformGetThreadID();
+    currentNode = threadContextMap;
 
-    ThreadContextMapNode *currentNode = threadContextMap;
     while (currentNode)
     {
         if (currentNode->threadID == threadID)

sezero avatar Jul 30 '23 23:07 sezero

Thanks so much! What a dumb mistake coming from C++.

S41L0R avatar Jul 31 '23 20:07 S41L0R

Yo! This isn't a plea to merge it or anything, (obviously that's up to you), I just wanted to give a little update by saying that this fork has been stable in my project thus far. (As it should be, it's a pretty simple addition, despite the large diff count.) Hopefully that inspires some confidence (: I know stability was probably a concern with this one, because of how big of a change it is.

I also know it deviates from the implementation planned for V4. That is, this does not include passing the instance/context as the first parameter to functions, and instead uses per-thread context-switching. I remember I had originally implemented something like what was planned for V4, but it ended up resulting in a lot of actual code needing to be rewritten. This context-switching approach meant that I didn't really get the opportunity to introduce bugs in the same way, as it's a simple replacement of references. (You can check the diff to see what I mean.)

Anyway, I write all this to inspire some confidence. I know you've learned not to trust external contributors with big stuff, but I really don't think this one is all that bad. When you start work on V4, I hope you fiddle around with it and potentially merge it to your working branch.

S41L0R avatar Apr 17 '24 01:04 S41L0R