Skip to content

pkg_editor: do not ignore failure to create directory #216

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

Closed
wants to merge 1 commit into from
Closed

pkg_editor: do not ignore failure to create directory #216

wants to merge 1 commit into from

Conversation

pcolberg
Copy link
Contributor

Resolves Coverity error "Unchecked return value from library (CHECKED_RETURN)".

@pcolberg pcolberg added the bug Something isn't working label Nov 26, 2022
@pcolberg pcolberg added this to the 2023.1 milestone Nov 26, 2022
@pcolberg pcolberg self-assigned this Nov 26, 2022
Resolves Coverity error "Unchecked return value from library (CHECKED_RETURN)".
@pcolberg pcolberg modified the milestones: 2023.1, 2023.2 Dec 23, 2022
@pcolberg
Copy link
Contributor Author

@IlanTruanovsky This was a first attempt at properly handling failures of mkdir/CreateDirectory. As you can see, however, this adds quite a bit of code including string buffer handling that itself can be a new source of bugs. I have since come to the conclusion that ignoring a failure to create the directory is a pragmatic solution; this will simply delay the failure to the first attempt of creating a file in the (not) newly created directory.

If you submit a pull request for this, you can simply ignore the return value with (void)mkdir(…) and expand the in-code comment to explain why this is a pragmatic solution (to avoid error-prone, platform-dependent error handling).

Taking a step back, cross-platform filesystem operations including proper error handling are nowadays provided in the C++17 filesystem library. However, so far we have not been able to make use of it since our minimum GCC version does not provide <filesystem>; it was only available as an experimental extension.

@pcolberg
Copy link
Contributor Author

Superseded by #247.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant