On Thu, Jul 13, 2023 at 09:53:58AM +0000, Tage Johansson wrote:
> Apologize if resending, but I'm not sure my previous email was
> actually delivered.
> 
> 
> On 7/12/2023 10:33 PM, Eric Blake wrote:
> 
> 
> >On Wed, Jul 12, 2023 at 03:19:59PM +0000, Tage Johansson wrote:
> >>Hello,
> >>
> >>
> >>While writing some tests for the Rust bindings, I discovered a
> >>memory leak
> >>with Valgrind due to a completion callback not being freed. More
> >>specificly,
> >>the completion callback of `aio_opt_info()` doesn't seem to be if
> >>`aio_opt_info()` failes. In this case, `aio_opt_info()` was called in the
> >>connected state, so it should indeed fail, but it should perhaps
> >>also call
> >>the free function associated with the callback.
> >Can you paste the valgrind output showing the leak?
> 
> 
> The Valgrind output is very Rust specific and only shows the Rust
> allocations which goes into the completion callback and are lost.
> 
> 
> But if you apply the following patch (which modifies
> tests/opt-info.c) you shall see that the completion callback is not
> called.
> 
> I have replaced a call to `nbd_opt_info()` with a call to
> `nbd_aio_opt_info()` and passed in a completion callback which just
> calls `exit(EXIT_FAILURE)`. So if the completion callback is called
> the test should fail, which it doesn't, at least not on my machine.

Isn't that OK?  Only .free is required to be called.

Rich.

> 
> diff --git a/tests/opt-info.c b/tests/opt-info.c
> index 86a0608..ba5c72b 100644
> --- a/tests/opt-info.c
> +++ b/tests/opt-info.c
> @@ -30,6 +30,12 @@
> 
>  #include <libnbd.h>
> 
> +int
> +my_completion_cb(void *_user_data, int *_err)
> +{
> +  exit(EXIT_FAILURE);
> +}
> +
>  int
>  main (int argc, char *argv[])
>  {
> @@ -201,7 +207,10 @@ main (int argc, char *argv[])
>      fprintf (stderr, "expecting export name 'good', got '%s'\n", s);
>      exit (EXIT_FAILURE);
>    }
> -  if (nbd_opt_info (nbd) != -1) {
> +  if (nbd_aio_opt_info (
> +        nbd,
> +        (nbd_completion_callback) { .callback = my_completion_cb }
> +      ) != -1) {
>      fprintf (stderr, "expecting error for opt_info\n");
>      exit (EXIT_FAILURE);
>    }
> 
> Best regards,
> 
> Tage
> 
> 

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW
_______________________________________________
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Reply via email to