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

do not mistake WAD-named directories for actual WAD files #2118

Merged
merged 2 commits into from
Jan 5, 2025

Conversation

fabiangreffrath
Copy link
Owner

Fixes a segfault when running the engine e.g. from the savegames directory.

Does anyone know the reason why we do not simply return false here?

woof/src/m_misc.c

Lines 50 to 56 in ec7344f

else
{
// If we can't open because the file is a directory, the
// "file" exists at least!
return errno == EISDIR;
}

@fabiangreffrath
Copy link
Owner Author

Does anyone know the reason why we do not simply return false here?

I mean, do we ever use M_FileExists() to check for the existence of a directory?

@fabiangreffrath
Copy link
Owner Author

I mean, do we ever use M_FileExists() to check for the existence of a directory?

Even worse, POSIX fopen() returns a valid FILE * pointer for a directory. How is it on Windows?

@fabiangreffrath
Copy link
Owner Author

Alternative approach:

--- a/src/m_misc.c
+++ b/src/m_misc.c
@@ -45,14 +45,11 @@ boolean M_FileExists(const char *filename)
     if (fstream != NULL)
     {
         fclose(fstream);
-        return true;
+        return M_DirExists(filename) == false;
     }
     else
     {
-        // If we can't open because the file is a directory, the
-        // "file" exists at least!
-
-        return errno == EISDIR;
+        return false;
     }
 }
 

@rfomin
Copy link
Collaborator

rfomin commented Jan 4, 2025

I mean, do we ever use M_FileExists() to check for the existence of a directory?

Even worse, POSIX fopen() returns a valid FILE * pointer for a directory. How is it on Windows?

I can't test it at the moment (winter holidays).

I like the PR, the only issue is that it would be different from Choco, so maybe we should rename the M_FileExists() function somehow.

@fabiangreffrath
Copy link
Owner Author

I like the PR

Do you mean the alternative approach that I posted before? I'll integrate it better into the rest of the code.

@rfomin
Copy link
Collaborator

rfomin commented Jan 4, 2025

I like the PR

Do you mean the alternative approach that I posted before? I'll integrate it better into the rest of the code.

Yes, alternative appoach is fine too.

@fabiangreffrath fabiangreffrath requested a review from rfomin January 5, 2025 11:04
@fabiangreffrath fabiangreffrath merged commit 7e39b8d into master Jan 5, 2025
8 checks passed
@fabiangreffrath fabiangreffrath deleted the fileexistsnotdir branch January 5, 2025 20:47
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.

2 participants