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

wolfssh: do cleanup in Curl_ssh_cleanup #11921

Closed
wants to merge 1 commit into from

Conversation

dfandrich
Copy link
Contributor

Closes: #11921

Copy link
Member

@bagder bagder left a comment

Choose a reason for hiding this comment

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

I don't think this is correct. The wolfSSH init and cleanup functions seem to be global for the application:

"Cleans up the wolfSSH library when done. Should be called at before termination of the application. After calling, do not make any more calls to the library."

@dfandrich
Copy link
Contributor Author

So, are you saying the call should rather be moved to main_free() in the curl tool or similiar and dropped altogether from the library? Because this change as-is doesn't change when it's being called.

@bagder
Copy link
Member

bagder commented Sep 23, 2023

I suggest they should be called from within curl_global_init and curl_global_cleanup

@jay
Copy link
Member

jay commented Sep 23, 2023

it's doing that he just changed it around
curl_global_cleanup -> Curl_ssh_cleanup -> wolfSSH_Cleanup
which is what we already do for the other ssh backends

@jay jay added the tidy-up label Sep 23, 2023
@bagder
Copy link
Member

bagder commented Sep 23, 2023

Ah yes, sorry I didn't read this properly from the beginning. Thanks for your patience. I'll go back to my corner and be quiet now.

@dfandrich dfandrich closed this in 739a9e8 Sep 24, 2023
@dfandrich dfandrich deleted the dfandrich/wolfsshclean branch September 24, 2023 06:16
ptitSeb pushed a commit to wasix-org/curl that referenced this pull request Sep 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

3 participants