[tlib][net] fix the memory leaks #7
Conversation
[tlib/src/net.cpp:69]: (error) Memory leak: buffer [tlib/src/net.cpp:168]: (error) Memory leak: buffer [tlib/src/net.cpp:189]: (error) Memory leak: buffer Found by https://github.com/bryongloden/cppcheck
| @@ -158,8 +159,8 @@ std::expected<tlib::packet> tlib::wait_for_packet(size_t socket_fd) { | |||
| : [socket] "g"(socket_fd), [buffer] "g"(reinterpret_cast<size_t>(buffer)) | |||
| : "rax", "rbx", "rcx"); | |||
|
|
|||
| free(buffer); | |||
There was a problem hiding this comment.
This is wrong, because buffer is returned by the system call aspayload and therefore is used then and returned by the function.
| @@ -179,8 +180,8 @@ std::expected<tlib::packet> tlib::wait_for_packet(size_t socket_fd, size_t ms) { | |||
| : [socket] "g"(socket_fd), [buffer] "g"(reinterpret_cast<size_t>(buffer)), [ms] "g"(ms) | |||
| : "rax", "rbx", "rcx"); | |||
|
|
|||
| free(buffer); | |||
There was a problem hiding this comment.
This is wrong too, for the same reason as line 162
| @@ -66,6 +66,7 @@ std::expected<tlib::packet> tlib::prepare_packet(size_t socket_fd, void* desc) { | |||
| : "rax", "rbx", "rcx", "rdx"); | |||
|
|
|||
| if (fd < 0) { | |||
| free(buffer); | |||
There was a problem hiding this comment.
This is absolutely correct here.
|
Hi @bryongloden Thank you very much for the Pull Request 👍 Memory leak are important bug indeed. And there are plenty, especially in the kernel itself. I know it and I plan to work on it, but I have to admit that it is not my absolute priority :) The first is indeed a crude memory leak. The two other are false positive, but it would have been difficult to find it since it comes from inside a system call. If you could fix your commit for the indicated review, I'll merge it inside the branch. Thanks Baptiste |
applied code review changes
|
Thanks for the update, I merged your change :) |
Greetings @wichtounet,
On line numbers
69,168, &189of net.cpp there are memory leaks, which are bugs.The reason we should
freeis that memory is a finite resource within our running programs. Sure in very short running simple programs, failing tofreememory won't have a noticeable effect. However on long running programs, failing tofreememory means we will be consuming a finite resource without replenishing it. Eventually it will run out and our program will abruptly crash. This is why we mustfreememory.