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
fix some memory leaks in the NBD server and enable asan for the tests #1374
Conversation
Also move some local variables to a smaller scope. Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
As we don't need the headers in GStrv form, remove that struct member and convert them to curl_slists immediately. Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
Now that the remaining memory leaks are fixed, we can enable address sanitizer in CI. Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1374 +/- ##
==========================================
- Coverage 79.95% 79.92% -0.04%
==========================================
Files 67 67
Lines 20057 20066 +9
==========================================
+ Hits 16037 16038 +1
- Misses 4020 4028 +8 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great to have the nbd c module asan-tested \o/
|
||
/* only read from the client on the first try */ | ||
if (!ctx->url) { | ||
g_autofree guint8 *data = g_malloc(xfer->request.len); | ||
g_autoptr(GVariant) v = NULL; | ||
g_auto(GVariantDict) dict = G_VARIANT_DICT_INIT(NULL); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe note that this changes from stack-allocated to heap-allocated and that the former variant lacked the g_variant_dict_clear()
call?
Also, the commit contains an unrelated newline change and unrelated code move. But since the result is still clear enough to read I'd be fine with that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is still stack-allocated, which is what g_auto
is used for.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ouh, right. My fault.
Most of the nbd asan tests are not running in the CI, as we're not using qemu there, yet.