Re: [Qemu-devel] [PATCH 2/4] qapi: output visitor crashes qemu if it encounters a NULL value

[Luiz Capitulino <lcapitulino@redhat.com>]

On Wed, 14 May 2014 20:29:37 +0300
Marcel Apfelbaum <marcel.a@redhat.com> wrote:

> On Wed, 2014-05-14 at 19:00 +0200, Andreas Färber wrote:
> > Am 13.05.2014 21:08, schrieb Eric Blake:
> > > On 05/13/2014 11:36 AM, Andreas Färber wrote:
> > >> Am 07.05.2014 16:42, schrieb Marcel Apfelbaum:
> > >>> A NULL value is not added to visitor's stack, but there is no
> > >>> check for that when the visitor tries to return that value,
> > >>> leading to Qemu crash.
> > >>> 
> > >>> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by:
> > >>> Marcel Apfelbaum <marcel.a@redhat.com>
> > >> 
> > >> Where does the Rb come from on this v1? Is it in any tree
> > >> already?
> > >> 
> > > 
> > > The (weak) R-b was here: 
> > > https://lists.gnu.org/archive/html/qemu-devel/2014-02/msg02861.html
> > 
> > Thanks.
> > > 
> > So Luiz was okay with it too, but his last message seems to be
> > indicating this needs to be fixed somewhere else, too:
> > 
> > https://lists.gnu.org/archive/html/qemu-devel/2014-02/msg05228.html
> > https://lists.gnu.org/archive/html/qemu-devel/2014-03/msg00217.html
> > 
> > Can/should that be addressed as a follow-up? Or is there a test case
> > that breaks?
> Simple and "popular" test case: the user does not use the -kernel-cmdline parameter.
> The patch is needed because otherwise the main function will fail
> if no value is passed by the user to string parameters. 
> 
> Regarding Luiz's concern, it can be a follow-up as I am not aware of
> any problem with that.

My concern was that I wasn't sure if this is the right fix for the issue
or if it's papering over the real bug. I quickly checked the code and it
seemed to make sense, but I didn't have time to study it deeper.

We could ask Michael Roth or Anthony, but I wouldn't hold this series
because of that. Here's my ACK if you need it:

Acked-by: Luiz Capitulino <lcapitulino@redhat.com>