[libvirt] [PATCH] conf: report error in virCPUDefParseXML

[Jincheng Miao]

On 09/25/2014 04:06 PM, Peter Krempa wrote:
> On 09/25/14 09:25, Jincheng Miao wrote:
>> On 09/25/2014 01:48 PM, Martin Kletzander wrote:
>>> On Thu, Sep 25, 2014 at 12:23:31PM +0800, Jincheng Miao wrote:
>>>> When detected invalid 'memAccess', virCPUDefParseXML should report
>>>> error.
>>>>
>>>> Resolves https://bugzilla.redhat.com/show_bug.cgi?id=1146334
>>>>
>>>> Signed-off-by: Jincheng Miao <jmiao at redhat.com>
>>>> ---
>>>> src/conf/cpu_conf.c | 4 ++--
>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
>>>> index 116aa58..a1081b9 100644
>>>> --- a/src/conf/cpu_conf.c
>>>> +++ b/src/conf/cpu_conf.c
>>>> @@ -510,13 +510,13 @@ virCPUDefParseXML(xmlNodePtr node,
>>>>                  def->cells[cur_cell].memAccess =
>>>>                      virMemAccessTypeFromString(memAccessStr);
>>>>
>>>> -                if (def->cells[cur_cell].memAccess <= 0) {
>>>> +                if ((int)def->cells[cur_cell].memAccess <= 0) {
>>> Any reason for the explicit cast in here?
>> Yes, I found enum virMemAccess will be convert to uint, like:
>>
>> (gdb) c
>> Continuing.
>>
>> Breakpoint 2, virCPUDefParseXML (node=<optimized out>,
>>      ctxt=ctxt at entry=0x7f92e80034a0, mode=mode at entry=VIR_CPU_TYPE_GUEST)
>>      at conf/cpu_conf.c:513
>> 513                    if (def->cells[cur_cell].memAccess <= 0) {
>> (gdb) whatis def->cells[cur_cell].memAccess
>> type = virMemAccess
>> (gdb) p def->cells[cur_cell].memAccess
>> $1 = 4294967295
>> (gdb) p def->cells[cur_cell].memAccess <= 0
>> $2 = 0
>>
>>
> In that case, use an intermediate variable with the correct type and
> assign it only after you verify that the value is correct.

Yes, you are right, it nonsense to set a unverified value to vm->def.
Version 2 will come up.

Thanks

Jincheng Miao

>
> Peter
>
>