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

Small memory leak fix in hostip.c #14147

Closed
wants to merge 2 commits into from
Closed

Small memory leak fix in hostip.c #14147

wants to merge 2 commits into from

Conversation

iCMDdev
Copy link
Contributor

@iCMDdev iCMDdev commented Jul 10, 2024

Hi!

First time looking at the curl source code, and I think I found a (most likely insignificant) memory leak.
Either way, I added a free(ca); to prevent memory leaks if Curl_inet_pton(AF_INET6, "::1", ipv6) fails in *get_localhost6(int port, const char *name).

@github-actions github-actions bot added the name lookup DNS and related tech label Jul 10, 2024
@bagder
Copy link
Member

bagder commented Jul 10, 2024

While this is "technically correct", in that if Curl_inet_pton() returns failure this would leak. But given this input, the function will not fail so it will never return anything but 1 for this code flow.

I rather suggest that we instead make the code reflect this reality, perhaps like this:

    (void)Curl_inet_pton(AF_INET6, "::1", ipv6);

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.

See comment

@iCMDdev
Copy link
Contributor Author

iCMDdev commented Jul 10, 2024

Makes sense! Yeah, that's a better choice.

This change better reflects functionality - `Curl_inet_pton` will not return failure in `*get_localhost6(int port, const char *name)`.
@bagder bagder closed this in c2e427c Jul 10, 2024
@bagder
Copy link
Member

bagder commented Jul 10, 2024

Thanks!

@iCMDdev
Copy link
Contributor Author

iCMDdev commented Jul 10, 2024

No problem, thanks for the commit!

@iCMDdev iCMDdev deleted the patch-1 branch July 10, 2024 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
name lookup DNS and related tech
Development

Successfully merging this pull request may close these issues.

2 participants