Skip to content

Commit

Permalink
(stdlib) Fix memory leak in tests
Browse files Browse the repository at this point in the history
This may seem minor and petty, but it causes a real problem now that we
start running the stdlib tests through Valgrind too. In other words, we
will do our very best to prevent *any* form of memory problems from
slipping into this code base. That's the theory, at least. :-)

https://gitlab.perlang.org/perlang/perlang/-/merge_requests/558
  • Loading branch information
perlun committed Nov 7, 2024
1 parent c7a4b9d commit 3baa650
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 16 deletions.
2 changes: 2 additions & 0 deletions release-notes/v0.6.0.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
- Return non-`const` strings [[!544][544]]
- Support dynamically resizing `StringBuilder` buffer [[!550][550]]
- Fix buffer overrun in dynamic resizing of `StringBuilder` [[!551][551]]
- Fix memory leak in tests [[!558][558]]

#### CI
- Add `dotnet test` CI job [[!499][499]]
Expand Down Expand Up @@ -145,3 +146,4 @@
[554]: https://gitlab.perlang.org/perlang/perlang/merge_requests/554
[555]: https://gitlab.perlang.org/perlang/perlang/merge_requests/555
[556]: https://gitlab.perlang.org/perlang/perlang/merge_requests/556
[558]: https://gitlab.perlang.org/perlang/perlang/merge_requests/558
30 changes: 14 additions & 16 deletions src/stdlib/test/print.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,26 +9,24 @@
#include "perlang_stdlib.h"

bool fwrite_mocked = false;
std::string* captured_output = nullptr;
std::string captured_output;

extern "C" void __real_fwrite(const void *__restrict __ptr, size_t __size, size_t __n, FILE *__restrict __s);

extern "C" void __wrap_fwrite(const void *__restrict __ptr, size_t __size, size_t __n, FILE *__restrict __s)
{
if (fwrite_mocked) {
// TODO: handle multiple lines of output. Possibly append to previous list?
if (captured_output != nullptr) {
delete captured_output;
}

// __ptr is not necessarily NUL-terminated at this point, so we must be careful to only copy the data that would
// have been printed to the file handle and then add a NUL terminator manually.
// have been printed to the file handle and then add a NUL terminator manually. Note that the use of a stack-
// allocated buffer here means that we would hypothetically fail on very long input.
unsigned long string_length = (__size * __n );
char* captured_output_buffer = (char *)malloc(string_length + 1);
char captured_output_buffer[string_length + 1];
strncpy(captured_output_buffer, (const char *)__ptr, string_length);
captured_output_buffer[string_length] = '\0';

captured_output = new std::string(captured_output_buffer);
// TODO: handle multiple lines of output. Possibly append to previous list?
captured_output.erase();
captured_output.append(captured_output_buffer);
}
else {
__real_fwrite(__ptr, __size, __n, __s);
Expand All @@ -41,7 +39,7 @@ TEST_CASE( "perlang::print float, 103.1f" )
perlang::print(103.1f);
fwrite_mocked = false;

REQUIRE(*captured_output == "103.1\n");
REQUIRE(captured_output == "103.1\n");
}

TEST_CASE( "perlang::print float, positive infinity" )
Expand All @@ -52,7 +50,7 @@ TEST_CASE( "perlang::print float, positive infinity" )
perlang::print(f);
fwrite_mocked = false;

REQUIRE(*captured_output == "Infinity\n");
REQUIRE(captured_output == "Infinity\n");
}

TEST_CASE( "perlang::print double, 123.45" )
Expand All @@ -61,7 +59,7 @@ TEST_CASE( "perlang::print double, 123.45" )
perlang::print(123.45);
fwrite_mocked = false;

REQUIRE(*captured_output == "123.45\n");
REQUIRE(captured_output == "123.45\n");
}

TEST_CASE( "perlang::print double, -46.0" )
Expand All @@ -70,7 +68,7 @@ TEST_CASE( "perlang::print double, -46.0" )
perlang::print(-46.0);
fwrite_mocked = false;

REQUIRE(*captured_output == "-46\n");
REQUIRE(captured_output == "-46\n");
}

TEST_CASE( "perlang::print double, 4294967296.123" )
Expand All @@ -79,7 +77,7 @@ TEST_CASE( "perlang::print double, 4294967296.123" )
perlang::print(4294967296.123);
fwrite_mocked = false;

REQUIRE(*captured_output == "4294967296.123\n");
REQUIRE(captured_output == "4294967296.123\n");
}

TEST_CASE( "perlang::print double, 4294967283" )
Expand All @@ -88,7 +86,7 @@ TEST_CASE( "perlang::print double, 4294967283" )
perlang::print(4294967283.0);
fwrite_mocked = false;

REQUIRE(*captured_output == "4294967283\n");
REQUIRE(captured_output == "4294967283\n");
}

TEST_CASE( "perlang::print double, 9223372036854775807" )
Expand All @@ -97,7 +95,7 @@ TEST_CASE( "perlang::print double, 9223372036854775807" )
perlang::print(9223372036854775807.0);
fwrite_mocked = false;

REQUIRE(*captured_output == "9.22337203685478E+18\n");
REQUIRE(captured_output == "9.22337203685478E+18\n");
}

// TODO: Make this test work. We need to (linker-)wrap puts to make it happen.
Expand Down

0 comments on commit 3baa650

Please sign in to comment.