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

Handling unrecoverable ABRT signals in HDF5 Library #126

Merged
merged 8 commits into from
Nov 8, 2024

Conversation

anthonypapetti
Copy link
Contributor

Fixes #118 #117 #84 #80

This commit adds error handling using std::signal to catch SIGABRT signals from the HDF5 Library in bag_dataset.cpp and exit the program gracefully.

@selimnairb
Copy link
Collaborator

@anthonypapetti Thanks for contributing this. The tests pass (except for the reporter, which I expect to fail since this is a PR; need to fix that to not run for PRs). Going to merge.

@selimnairb selimnairb merged commit 537fb5d into OpenNavigationSurface:master Nov 8, 2024
4 of 5 checks passed
@anthonypapetti
Copy link
Contributor Author

@selimnairb Found afterwards that this also fixes #81 , will update if I find any more that is fixed by it.

@anthonypapetti
Copy link
Contributor Author

anthonypapetti commented Nov 15, 2024

@selimnairb Found that it fixes #103 #79 #93

@@ -859,6 +859,7 @@ bool decodeDataQualityInfo(
else if (schemaVersion == 2)
{
//gmd:DQ_DataQuality/gmd:scope/gmd:DQ_Scope/gmd:level

Choose a reason for hiding this comment

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

Would it be better here to change BagDataQuality::scope to be a std::string instead of a char*? It would mean we wouldn't have to use delete[] manually everywhere, which might also help with bugs

@@ -1055,6 +1056,11 @@ std::tuple<double, double> Dataset::gridToGeo(
return {x, y};
}

void handleAbrt(int signum) {

Choose a reason for hiding this comment

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

I think this runs into some problems that @selimnairb and I talked about during the last meeting, I'm working on some code now to handle it "safely" within the specification. The worry is that using the std::cerr and streams allocates, which isn't well defined behavior within an abort handler. I wonder if there's a way that we can tell aborts apart from segfaults?

@anthonypapetti
Copy link
Contributor Author

Found that it fixes #96 #100 #102 #108

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.

oss-fuzz: opennavsurf-bag:bag_read_fuzzer: Abrt in H5FD_set_eoa
3 participants