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

Add error check after 'eosCreateTables()' in the 'eosSafeLoad()' function #322

Merged
merged 2 commits into from
Nov 28, 2023

Conversation

jhp-lanl
Copy link
Collaborator

@jhp-lanl jhp-lanl commented Nov 27, 2023

PR Summary

Fixes a bug when the comment table wasn't available. The eosCreateTables() function would return an error code, but the code wouldn't be handled. This change adds an early return to skip the eosLoadTables() call if eosCreateTables() fails.

EDIT - An additional check was needed for when the comment table is created but when the error code returned is okay. The comments table could still be invalid and so the length needs to be checked before trying to extract comments.

I think this should help address a bug found by @annapiegra where a comments table length would be negative when a comments table didn't exist, and would cause issues for the comments vector later on when the resize() member is called.

PR Checklist

  • Adds a test for any bugs fixed. Adds tests for new features.
  • Format your changes by using the make format command after configuring with cmake.
  • Document any new features, update documentation for changes made.
  • Make sure the copyright notice on any files you modified is up to date.
  • After creating a pull request, note it in the CHANGELOG.md file
  • If preparing for a new release, update the version in cmake.

@jhp-lanl jhp-lanl requested a review from annapiegra November 27, 2023 19:52
@annapiegra
Copy link
Collaborator

This doesn't solve the issue since commentLen is returned from eosSafeTableInfo. Initializing commentLen to 0 would be a possible solution (in the current implementation if the info returns an error commentLen keeps a garbage value)

@jhp-lanl
Copy link
Collaborator Author

This doesn't solve the issue since commentLen is returned from eosSafeTableInfo. Initializing commentLen to 0 would be a possible solution (in the current implementation if the info returns an error commentLen keeps a garbage value)

@annapiegra let me know if this fixes it

@jhp-lanl jhp-lanl requested a review from Yurlungur November 27, 2023 21:55
Copy link
Collaborator

@Yurlungur Yurlungur left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this @jhp-lanl this one is an interesting edge case.

eospac-wrapper/eospac_wrapper.cpp Show resolved Hide resolved
@jhp-lanl
Copy link
Collaborator Author

These were minor changes, but it still feels good to have the format check pass without having to run the format tool 😁

Copy link
Collaborator

@annapiegra annapiegra left a comment

Choose a reason for hiding this comment

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

Works now. Thanks!

@Yurlungur Yurlungur merged commit b598d4e into main Nov 28, 2023
4 checks passed
@Yurlungur Yurlungur deleted the jhp/eospac_comment branch November 28, 2023 17:23
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