[PATCH] libcmpiutil: Fix endianness issues in embedded object parsing
From: Thilo Boehm <tboehm@linux.vnet.ibm.com> The auxiliary functions _set_int_prop/parse_int_property only worked on little-endian archs as they performed an incorrect reinterpretation of 64bit integers. Fixed by using the proper CMPIValue union fields. Signed-off-by: Thilo Boehm <tboehm@linux.vnet.ibm.com> Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> --- eo_parser.c | 35 ++++++++++++++++++++++------------- eo_parser_xml.c | 49 +++++++++++++++++++++++++++++++++++++++---------- 2 files changed, 61 insertions(+), 23 deletions(-) diff --git a/eo_parser.c b/eo_parser.c index 36106fd..4c5b0ee 100644 --- a/eo_parser.c +++ b/eo_parser.c @@ -113,31 +113,40 @@ static int _set_int_prop(CMPISint64 value, CMPIInstance *inst) { CMPIStatus s; - uint64_t unsigned_val = 0; - int64_t signed_val = 0; + CMPIValue val; - switch(type) { + switch (type) { case CMPI_uint64: + val.uint64 = (uint64_t) value; + break; case CMPI_uint32: + val.uint32 = (uint32_t) value; + break; case CMPI_uint16: + val.uint16 = (uint16_t) value; + break; case CMPI_uint8: - unsigned_val = (uint64_t) value; - s = CMSetProperty(inst, - prop, - (CMPIValue *) &(unsigned_val), - type); + val.uint8 = (uint8_t) value; break; case CMPI_sint64: + val.sint64 = (int64_t) value; + break; case CMPI_sint32: + val.sint32 = (int32_t) value; + break; case CMPI_sint16: + val.sint16 = (int16_t) value; + break; case CMPI_sint8: + val.sint8 = (int8_t) value; + break; default: - signed_val = (int64_t) value; - s = CMSetProperty(inst, - prop, - (CMPIValue *) &(signed_val), - type); + return 0; } + s = CMSetProperty(inst, + prop, + &val, + type); if (s.rc == CMPI_RC_OK) return 1; diff --git a/eo_parser_xml.c b/eo_parser_xml.c index c8b28cc..234b04b 100644 --- a/eo_parser_xml.c +++ b/eo_parser_xml.c @@ -90,11 +90,48 @@ static CMPIType parse_int_property(const char *string, if (sign) { int64_t _val; ret = sscanf(string, "%" SCNi64, &_val); - val->sint64 = _val; + switch (size) { + case 8: + t = CMPI_sint8; + val->sint8 = (int8_t) _val; + break; + case 16: + t = CMPI_sint16; + val->sint16 = (int16_t) _val; + break; + case 32: + t = CMPI_sint32; + val->sint32 = (int32_t) _val; + break; + default: + case 64: + t = CMPI_sint64; + val->sint64 = (int64_t) _val; + break; + }; } else { uint64_t _val; ret = sscanf(string, "%" SCNu64, &_val); - val->uint64 = _val; + switch (size) { + case 8: + t = CMPI_uint8; + val->uint8 = (uint8_t) _val; + break; + case 16: + t = CMPI_uint16; + val->uint16 = (uint16_t) _val; + break; + case 32: + t = CMPI_uint32; + val->uint32 = (uint32_t) _val; + break; + default: + case 64: + t = CMPI_uint64; + val->uint64 = (uint64_t) _val; + break; + + }; } if (ret != 1) { @@ -102,14 +139,6 @@ static CMPIType parse_int_property(const char *string, return CMPI_null; } - switch (size) { - case 8: t = sign ? CMPI_sint8 : CMPI_uint8; break; - case 16: t = sign ? CMPI_sint16 : CMPI_uint16; break; - case 32: t = sign ? CMPI_sint32 : CMPI_uint32; break; - default: - case 64: t = sign ? CMPI_sint64 : CMPI_uint64; break; - }; - return t; } -- 1.7.9.5
On 08/08/2013 09:27 AM, Viktor Mihajlovski wrote:
From: Thilo Boehm <tboehm@linux.vnet.ibm.com>
The auxiliary functions _set_int_prop/parse_int_property only worked on little-endian archs as they performed an incorrect reinterpretation of 64bit integers. Fixed by using the proper CMPIValue union fields.
Signed-off-by: Thilo Boehm <tboehm@linux.vnet.ibm.com> Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> --- eo_parser.c | 35 ++++++++++++++++++++++------------- eo_parser_xml.c | 49 +++++++++++++++++++++++++++++++++++++++---------- 2 files changed, 61 insertions(+), 23 deletions(-)
Seems fine to me... I assume one author and one reviewer, right? I'd say push away. John
On 08/09/2013 12:25 AM, John Ferlan wrote:
On 08/08/2013 09:27 AM, Viktor Mihajlovski wrote:
From: Thilo Boehm <tboehm@linux.vnet.ibm.com>
The auxiliary functions _set_int_prop/parse_int_property only worked on little-endian archs as they performed an incorrect reinterpretation of 64bit integers. Fixed by using the proper CMPIValue union fields.
Signed-off-by: Thilo Boehm <tboehm@linux.vnet.ibm.com> Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> --- eo_parser.c | 35 ++++++++++++++++++++++------------- eo_parser_xml.c | 49 +++++++++++++++++++++++++++++++++++++++---------- 2 files changed, 61 insertions(+), 23 deletions(-)
Seems fine to me... I assume one author and one reviewer, right? Right, Thilo is the principal author and I have applied only minor editorial changes to the commit message (hence my signoff). I'd say push away. Thanks, but I fear I am depending on your kind help here ;-).
As a heads up: this patch is - so to speak - a portent of more (mainly but not only s390 arch related) to come. Unfortunately we were not able to make it in time for the recent release. -- Mit freundlichen Grüßen/Kind Regards Viktor Mihajlovski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martina Köderitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
On 08/09/2013 02:19 AM, Viktor Mihajlovski wrote:
On 08/09/2013 12:25 AM, John Ferlan wrote:
On 08/08/2013 09:27 AM, Viktor Mihajlovski wrote:
From: Thilo Boehm <tboehm@linux.vnet.ibm.com>
The auxiliary functions _set_int_prop/parse_int_property only worked on little-endian archs as they performed an incorrect reinterpretation of 64bit integers. Fixed by using the proper CMPIValue union fields.
Signed-off-by: Thilo Boehm <tboehm@linux.vnet.ibm.com> Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> --- eo_parser.c | 35 ++++++++++++++++++++++------------- eo_parser_xml.c | 49 +++++++++++++++++++++++++++++++++++++++---------- 2 files changed, 61 insertions(+), 23 deletions(-)
Seems fine to me... I assume one author and one reviewer, right? Right, Thilo is the principal author and I have applied only minor editorial changes to the commit message (hence my signoff). I'd say push away. Thanks, but I fear I am depending on your kind help here ;-).
As a heads up: this patch is - so to speak - a portent of more (mainly but not only s390 arch related) to come. Unfortunately we were not able to make it in time for the recent release.
I pushed the change after making an edit to remove the trailing white space in a few lines of eo_parser_xml.c (each of the case #: entries had trailing white space and git am complains). John
On 08/09/2013 12:38 PM, John Ferlan wrote:
I pushed the change after making an edit to remove the trailing white space in a few lines of eo_parser_xml.c (each of the case #: entries had trailing white space and git am complains).
thanks and sorry for the extra effort. I will be more careful in the future (never thought I'd be missing make syntax-check). -- Mit freundlichen Grüßen/Kind Regards Viktor Mihajlovski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martina Köderitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
participants (2)
-
John Ferlan -
Viktor Mihajlovski