Re: [pywbem-devel] [PATCH] Parsing of NULL values

[Jan S. <jsa...@re...>]

On 05/04/2012 03:42 PM, Klaus Kaempf wrote:
> * Jan Safranek <jsa...@re...> [May 03. 2012 15:56]:
> 
>> diff -up cmpi-bindings-0.4.17/swig/cmpi.i.old cmpi-bindings-0.4.17/swig/cmpi.i
>> --- cmpi-bindings-0.4.17/swig/cmpi.i.old	2012-05-03 10:24:05.275271659 +0200
>> +++ cmpi-bindings-0.4.17/swig/cmpi.i	2012-05-03 10:26:06.503726504 +0200
>> @@ -252,6 +252,9 @@ data_value(const CMPIData *dp)
>>    else if (dp->state & CMPI_badValue) {
>>      SWIG_exception(SWIG_ValueError, "bad value");
>>    }
>> +  else if (dp->state & CMPI_null) {
>> +    result = value_value(&(dp->value), CMPI_null);
>> +  }
>>    else if ((dp->type) & CMPI_ARRAY) {
>>      int size = CMGetArrayCount(dp->value.array, NULL);
>>      int i;
> 
> 
> Hmm, this fix looks wrong. CMPI_null is the type, not the state.
> Also, CMPI_null is defined as 0 (zero) so the 'if' expression is
> always false.
> 
> The current code (cmpi-binding 0.5.1) has
>   else if (dp->state & CMPI_nullValue) {
>     result = Target_Null;
>     Target_INCREF(result);
>   }
> with the correct 'if' expression and the same semantics as your patch.          
> 
> 
>> @@ -286,10 +290,6 @@ data_data(const CMPIData *dp)
>>    else if (dp->state & CMPI_badValue) {
>>      SWIG_exception(SWIG_ValueError, "bad value");
>>    }
>> -  else if (dp->state & CMPI_nullValue) {
>> -    result = Target_Null;
>> -    Target_INCREF(result);
>> -  }
>>    else if ((dp->type) & CMPI_ARRAY) {
>>      int size = CMGetArrayCount(dp->value.array, NULL);
>>      int i;
> 
> This patch is in cmpi-bindings 0.5.0 already.
> 
> 
>> diff -up cmpi-bindings-0.4.17/swig/python/cmpi_pywbem_bindings.py.orig cmpi-bindings-0.4.17/swig/python/cmpi_pywbem_bindings.py
>> --- cmpi-bindings-0.4.17/swig/python/cmpi_pywbem_bindings.py.orig	2012-05-03 13:21:14.982196890 +0200
>> +++ cmpi-bindings-0.4.17/swig/python/cmpi_pywbem_bindings.py	2012-05-03 13:22:18.684439933 +0200
>> @@ -860,6 +860,8 @@ class CMPIProxyProvider(object):
>>      def cmpi2pywbem_data(self, cdata, _type=None, is_array=None):
>>          #TODO check for valid cdata.state
>>          #TODO error handling
>> +        if (cdata.state & cmpi.CMPI_nullValue) > 0:
>> +            return None
>>          if _type is None:
>>              _type, is_array = _cmpi_type2string(cdata.type)
>>          attr = _type
> 
> I believe commit
> https://github.com/kkaempf/cmpi-bindings/commit/91c3eaa950f50b45cf89c1f2fbdbbdca51624322
> was wrong and reverting it should be equal to the above change.
> 
> 
>> diff -up cmpi-bindings-0.4.17/swig/cmpi.i.orig cmpi-bindings-0.4.17/swig/cmpi.i
>> --- cmpi-bindings-0.4.17/swig/cmpi.i.orig	2012-05-03 14:27:07.513117565 +0200
>> +++ cmpi-bindings-0.4.17/swig/cmpi.i	2012-05-03 14:28:11.160763227 +0200
>> @@ -289,7 +289,7 @@ data_data(const CMPIData *dp)
>>    else if (dp->state & CMPI_badValue) {
>>      SWIG_exception(SWIG_ValueError, "bad value");
>>    }
>> -  else if ((dp->type) & CMPI_ARRAY) {
>> +  else if (!(dp->state & CMPI_nullValue) && (dp->type & CMPI_ARRAY)) {
>>      int size = CMGetArrayCount(dp->value.array, NULL);
>>      int i;
>>      result = Target_SizedArray(size);
> 
> This fix is also obsolete since its based on the old implementation of
> data_data(). 
> The old data_data() falsely converted a CMPIData structure to a value
> of the target language.
> The fixed data_data() correctly converts a CMPIData structure (C
> language level) to an instance of CMPIData class (target language
> level).
> Such CMPIData instance can then be checked e.g. for NULL value by
> using is_null().

Ok, I checked that current cmpi-bindings release (0.5.2) works well for
me. Thanks a lot!

BTW, what is the official way of sending bugs/patches? Should I use
https://github.com/kkaempf/cmpi-bindings/issues or this list?

Jan