2:35 a.m.
2011/4/27 Daniel Veillard <veillard(a)redhat.com>:
On Mon, Apr 25, 2011 at 01:32:43PM +0200, Matthias Bolte wrote:
> 2011/4/25 Matthias Bolte <matthias.bolte(a)googlemail.com>:
> > This was broken by the refactoring in ac1e6586ec75. It resulted in a
> > segfault for 'virsh vol-dumpxml' and related volume functions.
> >
>
> Actually that patch doesn't fix the problem correctly. It just turns
> the segfault into an error message.
>
> Here's v2 that really fixes the problem. Note the important difference
> between anyType->_type and anyType->type :)
>
> Matthias
> From a17245fb92c88439ffbb84e34bebf1afb6903885 Mon Sep 17 00:00:00 2001
> From: Matthias Bolte <matthias.bolte(a)googlemail.com>
> Date: Mon, 25 Apr 2011 12:38:17 +0200
> Subject: [PATCH] esx: Fix dynamic dispatch for CastFromAnyType functions
>
> This was broken by the refactoring in ac1e6586ec75. It resulted in a
> segfault for 'virsh vol-dumpxml' and related volume functions.
>
> Before the refactoring all users of the ESX_VI__TEMPLATE__DISPATCH
> macro dispatched on the item type, as the item is the input to all those
> functions.
>
> Commit ac1e6586ec75 made the dynamically dispatched CastFromAnyType
> functions use this macro too, but this functions dispatched on the
> actual type of the AnyType object. The item is the output of the
> CastFromAnyType functions.
>
> This difference was missed in the refactoring, making CastFromAnyType
> functions dereferencing the item pointer that is NULL at the time of
> the dispatch.
> ---
> src/esx/esx_vi_types.c | 14 ++++++++------
> 1 files changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/src/esx/esx_vi_types.c b/src/esx/esx_vi_types.c
> index dd83954..3c81021 100644
> --- a/src/esx/esx_vi_types.c
> +++ b/src/esx/esx_vi_types.c
> @@ -533,8 +533,9 @@
> * Macros to implement dynamic dispatched functions
> */
>
> -#define ESX_VI__TEMPLATE__DISPATCH(__type, _dispatch, _error_return) \
> - switch (item->_type) { \
> +#define ESX_VI__TEMPLATE__DISPATCH(_actual_type, __type, _dispatch, \
> + _error_return) \
> + switch (_actual_type) { \
> _dispatch \
> \
> case esxVI_Type_##__type: \
> @@ -543,7 +544,7 @@
> default: \
> ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, \
> _("Call to %s for unexpected type '%s'"),
__FUNCTION__, \
> - esxVI_Type_ToString(item->_type)); \
> + esxVI_Type_ToString(_actual_type)); \
> return _error_return; \
> }
>
> @@ -585,7 +586,8 @@
>
> #define ESX_VI__TEMPLATE__DYNAMIC_FREE(__type, _dispatch, _body) \
> ESX_VI__TEMPLATE__FREE(__type, \
> - ESX_VI__TEMPLATE__DISPATCH(__type, _dispatch, /* nothing */) \
> + ESX_VI__TEMPLATE__DISPATCH(item->_type, __type, _dispatch, \
> + /* nothing */) \
> _body)
>
>
> @@ -618,14 +620,14 @@
>
> #define ESX_VI__TEMPLATE__DYNAMIC_CAST_FROM_ANY_TYPE(__type, _dispatch) \
> ESX_VI__TEMPLATE__CAST_FROM_ANY_TYPE_EXTRA(__type, esxVI_##__type, \
> - ESX_VI__TEMPLATE__DISPATCH(__type, _dispatch, -1), \
> + ESX_VI__TEMPLATE__DISPATCH(anyType->type, __type, _dispatch, -1), \
> /* nothing */)
>
>
>
> #define ESX_VI__TEMPLATE__DYNAMIC_SERIALIZE(__type, _dispatch, _serialize) \
> ESX_VI__TEMPLATE__SERIALIZE_EXTRA(__type, \
> - ESX_VI__TEMPLATE__DISPATCH(__type, _dispatch, -1), \
> + ESX_VI__TEMPLATE__DISPATCH(item->_type, __type, _dispatch, -1), \
> _serialize)
>
Not sure I fully understand but looks regular and okay, ACK :-)
Daniel
Actually, this shows that even I'm not always aware of all the details
in the ESX driver, otherwise this regression fix wouldn't have be
necessary and I wouldn't have needed two attempts to get it right. So
don't feel too bad about not fully understanding it :)
This is probably a sign that I should try to reduce the complexity and
levels of indirection in the generated code and the code generator.
Thanks, finally pushed v2.
Matthias