Re: [Qemu-devel] [PATCH for-2.10 1/4] stubs: Add vm state change handler stubs

[Fam Zheng <famz@redhat.com>]

On Tue, 08/15 07:26, Eric Blake wrote:
> On 08/14/2017 11:04 PM, Fam Zheng wrote:
> > Signed-off-by: Fam Zheng <famz@redhat.com>
> 
> A bit sparse on the 'why' - presumably, upcoming patches will fail to
> compile if the stub is not present, but mentioning what dependency this
> solves never hurts.
> 
> > ---
> >  stubs/Makefile.objs          |  1 +
> >  stubs/change-state-handler.c | 14 ++++++++++++++
> >  2 files changed, 15 insertions(+)
> >  create mode 100644 stubs/change-state-handler.c
> > 
> 
> > +++ b/stubs/change-state-handler.c
> > @@ -0,0 +1,14 @@
> > +#include "qemu/osdep.h"
> > +#include "qemu-common.h"
> > +#include "sysemu/sysemu.h"
> > +
> > +VMChangeStateEntry *qemu_add_vm_change_state_handler(VMChangeStateHandler *cb,
> > +                                                     void *opaque)
> > +{
> > +    return g_malloc0(1);
> > +}
> 
> Hmm - this is NOT a VMChangeStateEntry; if it ever gets dereferenced,
> the caller is (probably) accessing memory out of bounds.  Presumably,
> since it is a stub, this should never be called - and if that's the
> case, can we just get away with returning NULL instead (I'd rather have
> the caller SEGFAULT than dereference out-of-bounds into the heap, if
> this stub gets used inappropriately).

Good point, will update this patch.

> 
> > +
> > +void qemu_del_vm_change_state_handler(VMChangeStateEntry *e)
> > +{
> > +    g_free(e);
> 
> And of course, if you don't allocate anything, this can be a no-op.
> 
> > +}
> > 
> 

Fam