Skip to content

test: add bounds check for nfs test #4176

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

Conversation

bwbarrett
Copy link
Member

The nfs test builds a list of entries for all mount points on the
system, and then checks each mount point for expected results
against the opal_path_nfs() call. This test assumed that there
were less than 256 mount points on the system, which apparently
isn't true on some Cray systems. This patch makes two changes:
add the proper bound check to avoid a segmentation fault and
double the list of mount points we'll return.

Signed-off-by: Brian Barrett [email protected]

The nfs test builds a list of entries for all mount points on the
system, and then checks each mount point for expected results
against the opal_path_nfs() call.  This test assumed that there
were less than 256 mount points on the system, which apparently
isn't true on some Cray systems.  This patch makes two changes:
add the proper bound check to avoid a segmentation fault and
double the list of mount points we'll return.

Signed-off-by: Brian Barrett <[email protected]>
@ggouaillardet
Copy link
Contributor

i incidentally noticed dirs_tmp is not correctly allocated, it should be

dirs_tmp = (char**) calloc (MAX_DIR, sizeof(char*));

(instead of sizeof(char **))

should we realloc() instead of using a larger MAX_DIR ?
so we do not get bitten again by systems with more than 512 mount points

Copy link
Member

@jsquyres jsquyres left a comment

Choose a reason for hiding this comment

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

This PR is a minimum-distance fix -- it solves the problem (even if there are more than [gulp] 512 mount points). Just for the helluvit, I filed #4179 to go slightly beyond this PR -- it counts how many mount points there are before reading them in. Hence, we'll always check all mount points (even if there are more than 512).

I don't really care which one of these 2 PRs are merged -- the "count" solution was on my brain, so I filed #4179. But both this PR and #4179 adequately solve the problem.

@jsquyres
Copy link
Member

jsquyres commented Sep 6, 2017

#4179 was taken over this one. Closing.

@jsquyres jsquyres closed this Sep 6, 2017
@bwbarrett bwbarrett deleted the nfs-fix branch May 8, 2020 00:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants