Skip to content

Conversation

@phoemur
Copy link

@phoemur phoemur commented Sep 19, 2017

As you know, plain C has not RAII, so the structure created in

struct curl_slist *chunk = NULL;

Is never deallocated by the underlying library. I opted in this pull request to let this structure as a private member and clean in the destructor:
https://github.com/phoemur/tinycurl/blob/b95a68f84ecc171638d00b5de688869f9d7493f3/tinycurl.cpp#L24

Also improved the resource creation in the constructor and better cleanup in this pull request.
This small change seems to make a huge difference, as you can see:

Valgrind before changes:
`
==3320==

==3320== HEAP SUMMARY:

==3320==     in use at exit: 96,824 bytes in 3,233 blocks

==3320==   total heap usage: 4,324 allocs, 1,091 frees, 391,637 bytes allocate

==3320==

==3320== LEAK SUMMARY:

==3320==    definitely lost: 0 bytes in 0 blocks

==3320==    indirectly lost: 0 bytes in 0 blocks

==3320==      possibly lost: 0 bytes in 0 blocks

==3320==    still reachable: 96,824 bytes in 3,233 blocks

==3320==         suppressed: 0 bytes in 0 blocks

==3320== Reachable blocks (those to which a pointer was found) are not shown.

==3320== To see them, rerun with: --leak-check=full --show-leak-kinds=all

==3320== 

==3320== For counts of detected and suppressed errors, rerun with: -v

==3320== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

`

Valgrind After changes:
`
==3419==

==3419== HEAP SUMMARY:

==3419==     in use at exit: 528 bytes in 10 blocks

==3419==   total heap usage: 4,324 allocs, 4,314 frees, 391,637 bytes allocated

==3419== 

==3419== LEAK SUMMARY:

==3419==    definitely lost: 0 bytes in 0 blocks

==3419==    indirectly lost: 0 bytes in 0 blocks

==3419==      possibly lost: 0 bytes in 0 blocks

==3419==    still reachable: 528 bytes in 10 blocks

==3419==         suppressed: 0 bytes in 0 blocks

==3419== Reachable blocks (those to which a pointer was found) are not shown.

==3419== To see them, rerun with: --leak-check=full --show-leak-kinds=all

==3419== 

==3419== For counts of detected and suppressed errors, rerun with: -v

==3419== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)`

I could not eliminate all leaks, though going from 96,824 to 528 bytes in use at exit semmed worth a pull request.

Thanks for your attention

@phoemur
Copy link
Author

phoemur commented Sep 19, 2017

Also noted that the remaining bytes left behind here allocated by curl itself through libcrypto:
CRYPTO_malloc (in /lib64/libcrypto.so.1.0.0)

So there's not much we can do about it.

@phoemur phoemur mentioned this pull request Sep 19, 2017
std::string m_url;
std::string m_data;
std::string m_upload_data;
struct curl_slist *opt_list = NULL;
Copy link
Owner

Choose a reason for hiding this comment

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

I'm not sure this will work with C++ 98

Copy link
Author

Choose a reason for hiding this comment

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

I have no C++98 compiler at hand, but since this is libcurl interface,which is plain old C, maybe its worth a try...

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.

2 participants