Re: [Qemu-devel] [PATCH 11/37] tests: fix small leak in test-io-channel-command

[Marc-André Lureau]

Hi

On Wed, Jul 20, 2016 at 1:16 AM, Marc-André Lureau <address@hidden> wrote:
> Hi
>
> ----- Original Message -----
>> On 07/19/2016 02:54 AM, address@hidden wrote:
>> > From: Marc-André Lureau <address@hidden>
>> >
>> > srcfifo && dstfifo must still be freed in this case.
>> >
>> > Signed-off-by: Marc-André Lureau <address@hidden>
>> > ---
>> >  tests/test-io-channel-command.c | 3 ++-
>> >  1 file changed, 2 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/tests/test-io-channel-command.c
>> > b/tests/test-io-channel-command.c
>> > index 1d1f461..95be370 100644
>> > --- a/tests/test-io-channel-command.c
>> > +++ b/tests/test-io-channel-command.c
>> > @@ -40,7 +40,7 @@ static void test_io_channel_command_fifo(bool async)
>> >
>> >      unlink(TEST_FIFO);
>> >      if (access("/bin/socat", X_OK) < 0) {
>> > -        return; /* Pretend success if socat is not present */
>> > +        goto end; /* Pretend success if socat is not present */
>> >      }
>>
>> If we fail here...
>>
>> >      if (mkfifo(TEST_FIFO, 0600) < 0) {
>>
>> ...then we don't create a fifo here...
>>
>> >          abort();
>> > @@ -59,6 +59,7 @@ static void test_io_channel_command_fifo(bool async)
>> >      object_unref(OBJECT(src));
>> >      object_unref(OBJECT(dst));
>> >
>> > +end:
>> >      g_free(srcfifo);
>> >      g_free(dstfifo);
>> >      unlink(TEST_FIFO);
>>
>> ...and unlink() will (hopefully) fail to unlink a missing file.  But in
>> the worst case, it unlinks someone else's file.  Probably worth being a
>> bit stricter about only undoing what you have already done.
>
> But the test starts by unlinking unconditionally too, so not sure it's really 
> worth.

I fixed the test by using mkdtemp() instead, which also permits
running tests concurrently.

-- 
Marc-André Lureau