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 missing error string in ABT_error_get_str #50

Merged
merged 1 commit into from Apr 6, 2018

Conversation

bcumming
Copy link
Contributor

@bcumming bcumming commented Mar 30, 2018

Add missing error string for ABT_ERR_INV_MUTEX_ATTR to the lookup
table for error strings.

This fixes a bug that lead to:

  • segmentation fault when ABT_error_str is called with err==ABT_ERR_FEATURE_NA
  • incorrect error strings for err>ABT_ERR_INV_MUTEX

@bcumming
Copy link
Contributor Author

bcumming commented Mar 30, 2018

This fix adds the missing string, ABT_ERR_INV_MUTEX_ATTR to the lookup table.

However, to guard against segmentation faults in the future if the table is out of date,
the range check:

    ABTI_CHECK_TRUE(err >= ABT_SUCCESS && err <= ABT_ERR_FEATURE_NA,
                    ABT_ERR_OTHER);

could check against the range of the array:

     ABTI_CHECK_TRUE(err >= 0 && err < sizeof(err_str)/sizeof(char*),                                                                                                
                     ABT_ERR_OTHER);

This would ensure you don't ever go out of bounds.

The error string for ABT_ERR_INV_MUTEX_ATTR was missing in  the lookup
table for error strings.

This lead to
 * segmentation fault when ABT_error_str is called with err=ABT_ERR_FEATURE_NA
 * incorrect error strings for err>ABT_ERR_INV_MUTEX
@halimamer
Copy link

test:jenkins

@halimamer
Copy link

@bcumming , thanks for contributing the patch. I will merge it in a minute.
Regarding the array out of bounds issue, as long as the array and macros are kept in sync, both checks are equivalent. If a mismatch exists, even checking the array bounds might not be enough since the macro values might be inconsistent. I am looking into using an enum instead of macros. This will allow checking for the bounds of the array to match the number of distinct elements of the enum as well as out of bounds accesses.

@halimamer halimamer merged commit 054226b into pmodels:master Apr 6, 2018
@carns
Copy link
Contributor

carns commented Apr 6, 2018

FYI, in the future it might be worth looking into X macros for this sort of thing.

We use them for a similar purpose in Darshan (e.g. https://xgitlab.cels.anl.gov/darshan/darshan/blob/master/darshan-log-format.h#L123) when we need to keep some arrays in sync.

@halimamer
Copy link

Thank you for the suggestion @carns , I will look into it.

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.

None yet

3 participants