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

[Markus Armbruster]

Luiz Capitulino <address@hidden> writes:

> On Wed, 14 May 2014 20:29:37 +0300
> Marcel Apfelbaum <address@hidden> 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 <address@hidden> Signed-off-by:
>> > >>> Marcel Apfelbaum <address@hidden>
>> > >> 
>> > >> 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.

I can have a look tomorrow.

> 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 <address@hidden>