Skip to content

spawn with info: do not erroneously free(tmp) #8777

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

Merged
merged 1 commit into from
Apr 6, 2021

Conversation

abouteiller
Copy link
Member

@abouteiller abouteiller commented Apr 6, 2021

In some cases we would issue free(tmp) on values that have not been created from asprintf, thus crashing

Signed-off-by: Aurelien Bouteiller [email protected]

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.

Can you update the commit message and PR title to something declarative? E.g.:

spawn with info: do not erroneously free(tmp)

In some cases we would issue free(tmp) on values that have not been
created from asprintf, thus crashing

Signed-off-by: Aurelien Bouteiller <[email protected]>
@abouteiller abouteiller changed the title spawn with info: we would issue free(tmp) on values that have not been allocated spawn with info: do not erroneously free(tmp) Apr 6, 2021
@abouteiller
Copy link
Member Author

bot:retest

@awlauria
Copy link
Contributor

awlauria commented Apr 6, 2021

I would like to see the scope of tmp more limited, since it looks like it's used in only two spots. But that's outside the scope of this pr.

@awlauria
Copy link
Contributor

awlauria commented Apr 6, 2021

@abouteiller can you please cherry-pick to v5.0.x?

@awlauria awlauria merged commit 470eaca into open-mpi:master Apr 6, 2021
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.

4 participants