[Qemu-devel] [PATCH v5 0/5] Memory leak fixes

Marc-André Lureau posted 5 patches 6 years, 11 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20170503223846.6559-1-marcandre.lureau@redhat.com
Test checkpatch failed
Test docker passed
Test s390x passed
audio/audio.c       | 2 ++
audio/wavcapture.c  | 1 +
memory_mapping.c    | 1 +
slirp/socket.c      | 3 +++
tests/test-keyval.c | 4 ++++
5 files changed, 11 insertions(+)
[Qemu-devel] [PATCH v5 0/5] Memory leak fixes
Posted by Marc-André Lureau 6 years, 11 months ago
Hi,

A new series of leaks spotted by ASAN. Mostly after introducing of the
test-hmp. Would it be useful having a configure --enable-asan, and
enabled by default with --enable-debug?

Marc-André Lureau (5):
  test-keyval: fix leaks
  audio: fix capture buffer leaks
  audio: fix WAVState leak
  slirp: fix leak
  dump: fix memory_mapping_filter leak

 audio/audio.c       | 2 ++
 audio/wavcapture.c  | 1 +
 memory_mapping.c    | 1 +
 slirp/socket.c      | 3 +++
 tests/test-keyval.c | 4 ++++
 5 files changed, 11 insertions(+)

-- 
2.12.0.191.gc5d8de91d


Re: [Qemu-devel] [PATCH v5 0/5] Memory leak fixes
Posted by Marc-André Lureau 6 years, 11 months ago
This is actually v1 (confused with git-publish)

----- Original Message -----
> Hi,
> 
> A new series of leaks spotted by ASAN. Mostly after introducing of the
> test-hmp. Would it be useful having a configure --enable-asan, and
> enabled by default with --enable-debug?
> 
> Marc-André Lureau (5):
>   test-keyval: fix leaks
>   audio: fix capture buffer leaks
>   audio: fix WAVState leak
>   slirp: fix leak
>   dump: fix memory_mapping_filter leak
> 
>  audio/audio.c       | 2 ++
>  audio/wavcapture.c  | 1 +
>  memory_mapping.c    | 1 +
>  slirp/socket.c      | 3 +++
>  tests/test-keyval.c | 4 ++++
>  5 files changed, 11 insertions(+)
> 
> --
> 2.12.0.191.gc5d8de91d
> 
> 

Re: [Qemu-devel] [PATCH v5 0/5] Memory leak fixes
Posted by no-reply@patchew.org 6 years, 11 months ago
Hi,

This series seems to have some coding style problems. See output below for
more information:

Subject: [Qemu-devel] [PATCH v5 0/5] Memory leak fixes
Message-id: 20170503223846.6559-1-marcandre.lureau@redhat.com
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
84762e2 dump: fix memory_mapping_filter leak
345a5d8 slirp: fix leak
3f1ae28 audio: fix WAVState leak
565e071 audio: fix capture buffer leaks
2ce5984 test-keyval: fix leaks

=== OUTPUT BEGIN ===
Checking PATCH 1/5: test-keyval: fix leaks...
Checking PATCH 2/5: audio: fix capture buffer leaks...
ERROR: space prohibited between function name and open parenthesis '('
#22: FILE: audio/audio.c:2031:
+                g_free (cap->hw.mix_buf);

ERROR: space prohibited between function name and open parenthesis '('
#23: FILE: audio/audio.c:2032:
+                g_free (cap->buf);

total: 2 errors, 0 warnings, 8 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 3/5: audio: fix WAVState leak...
ERROR: space prohibited between function name and open parenthesis '('
#22: FILE: audio/wavcapture.c:91:
+    g_free (wav);

total: 1 errors, 0 warnings, 7 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 4/5: slirp: fix leak...
ERROR: suspect code indent for conditional statements (2, 6)
#37: FILE: slirp/socket.c:103:
+  if (so->so_tcpcb) {
+      free(so->so_tcpcb);

total: 1 errors, 0 warnings, 9 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 5/5: dump: fix memory_mapping_filter leak...
=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org
Re: [Qemu-devel] [PATCH v5 0/5] Memory leak fixes
Posted by Alex Bennée 6 years, 11 months ago
Marc-André Lureau <marcandre.lureau@redhat.com> writes:

> Hi,
>
> A new series of leaks spotted by ASAN. Mostly after introducing of the
> test-hmp. Would it be useful having a configure --enable-asan, and

Yes.

> enabled by default with --enable-debug?

Not sure - isn't --enable-debug just for debug symbols and we enable
extra stuff like --enable-tcg-debug for additional checking? Given the
overhead of asan I still think we need a configure setup for reasonably
debugable but still performant.

>
> Marc-André Lureau (5):
>   test-keyval: fix leaks
>   audio: fix capture buffer leaks
>   audio: fix WAVState leak
>   slirp: fix leak
>   dump: fix memory_mapping_filter leak
>
>  audio/audio.c       | 2 ++
>  audio/wavcapture.c  | 1 +
>  memory_mapping.c    | 1 +
>  slirp/socket.c      | 3 +++
>  tests/test-keyval.c | 4 ++++
>  5 files changed, 11 insertions(+)


--
Alex Bennée

Re: [Qemu-devel] [PATCH v5 0/5] Memory leak fixes
Posted by Paolo Bonzini 6 years, 11 months ago

On 04/05/2017 00:38, Marc-André Lureau wrote:
> Hi,
> 
> A new series of leaks spotted by ASAN. Mostly after introducing of the
> test-hmp. Would it be useful having a configure --enable-asan, and
> enabled by default with --enable-debug?

Yes, --enable-asan would be a nice idea, and we can also add a build
target for Docker tests.

Paolo