Skip to content

fix #381 #382

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

fix #381 #382

wants to merge 3 commits into from

Conversation

foryoung365
Copy link

std::fstream fs(simplePath, std::ios::app);

File existence check using fstream with ios::app is implemented as a workaround because:

  • ifstream::good() incorrectly handles directories on Unix-like systems
  • <sys/stat.h> adds platform-specific dependencies
  • It avoids duplication with CppCheck's 'path' implementation
  • std::filesystem (the proper solution) requires C++17 support

simplecpp.cpp Outdated

// This is a workaround to properly check if a file exists,
// since std::ifstream::good() incorrectly returns true for directories
std::fstream fs(simplePath, std::ios::app);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is an interesting handling.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This solution still has flaws; when the file does not exist, an empty file will be created. Therefore, I replaced it with

std::fstream fs(simplePath, std::ios::in|std::ios::out);

Here is a simple test result (linux, mac, and windows):
https://github.com/foryoung365/CppFstreamTest/actions/runs/11549258974

Here are some references:
https://en.cppreference.com/w/cpp/io/basic_filebuf/open

@firewave
Copy link
Collaborator

Thanks. Please give me a few days since I might be out sick.

@danmar
Copy link
Owner

danmar commented Dec 5, 2024

how do we test this? you can easily check if a file exists or not in a unit test but if you need to test various edge cases I guess a pytest working in a tempdir is better.

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.

3 participants