Re: [Qemu-devel] [PATCH for-2.5 3/3] vhost-user-test: fix crash with glib < 2.36

[Marc-André Lureau]

On Mon, Nov 30, 2015 at 2:09 PM, Michael S. Tsirkin <address@hidden> wrote:
> On Mon, Nov 30, 2015 at 06:00:29AM -0500, Marc-André Lureau wrote:
>>
>>
>> ----- Original Message -----
>> > On Fri, Nov 27, 2015 at 03:41:20PM +0100, address@hidden wrote:
>> > > From: Marc-André Lureau <address@hidden>
>> > >
>> > > The prepare callback needs to be implemented with glib < 2.36.
>> > >
>> > > Signed-off-by: Marc-André Lureau <address@hidden>
>> >
>> > But nothing prevents us from implementing it for all versions.
>>
>> Sure, it's just a kind of dead code for me that should be removed in future 
>> version. So I prefer to keep it under #ifdef.
>>
>> > > ---
>> > >  tests/vhost-user-test.c | 13 ++++++++++++-
>> > >  1 file changed, 12 insertions(+), 1 deletion(-)
>> > >
>> > > diff --git a/tests/vhost-user-test.c b/tests/vhost-user-test.c
>> > > index 29205ed..4ab48e4 100644
>> > > --- a/tests/vhost-user-test.c
>> > > +++ b/tests/vhost-user-test.c
>> > > @@ -506,8 +506,19 @@ test_migrate_source_check(GSource *source)
>> > >      return FALSE;
>> > >  }
>> > >
>> > > +#if !GLIB_CHECK_VERSION(2,36,0)
>> > > +static gboolean
>> > > +test_migrate_source_prepare(GSource *source, gint *timeout)
>> > > +{
>> > > +    *timeout = -1;
>> > > +    return FALSE;
>> > > +}
>> > > +#endif
>> > > +
>> > >  GSourceFuncs test_migrate_source_funcs = {
>> > > -    NULL,
>> > > +#if !GLIB_CHECK_VERSION(2,36,0)
>> > > +    test_migrate_source_prepare,
>> > > +#endif
>> > >      test_migrate_source_check,
>> >
>> > So now for version 2.36 check will be used instead of prepare.
>> > That's clearly wrong.
>>
>> instead?
>> >
>> > I'll switch this to named initializers to avoid the mess.
>>
>> I don't follow what is the mess here.
>
>
> You sent v2 that fixes it, and my patch fixes it too, so we don't need to 
> argue.


I fixed it, you changed it because you don't agree with my will to
keep the version check to remove the dumb callback in the future. I
disagree, I think this is a right way to cleanup the code in the
future. Btw, Author/ Reported-by is wrong: it is the other way around.
You reported it to me, and I fixed it. Please reconsider my patch.

-- 
Marc-André Lureau