On 11/12/2013 10:28 PM, John Ferlan wrote: > On 10/11/2013 07:47 AM, Viktor Mihajlovski wrote: >> From: Thilo Boehm <[email protected]> >> >> A number of CIM properties was set in an endianness-unsafe manner >> leading to failures on big endian systems. >> >> Signed-off-by: Thilo Boehm <[email protected]> >> Signed-off-by: Viktor Mihajlovski <[email protected]> >> --- >> src/Virt_FilterEntry.c | 46 ++++++++++++++++++++++++---------------------- >> 1 file changed, 24 insertions(+), 22 deletions(-) >> >> diff --git a/src/Virt_FilterEntry.c b/src/Virt_FilterEntry.c >> index b7042da..e41b4b6 100644 >> --- a/src/Virt_FilterEntry.c >> +++ b/src/Virt_FilterEntry.c >> @@ -59,8 +59,8 @@ struct rule_data_t { >> const char *dstportend; >> }; >> >> -static int octets_from_mac(const char * s, unsigned int *buffer, >> - unsigned int size) >> +static int octets_from_mac(const char * s, uint8_t *buffer, >> + unsigned int size) >> { >> unsigned int _buffer[6]; >> unsigned int i, n = 0; >> @@ -86,8 +86,8 @@ static int octets_from_mac(const char * s, unsigned int >> *buffer, >> return n; >> } >> >> -static int octets_from_ip(const char * s, unsigned int *buffer, >> - unsigned int size) >> +static int octets_from_ip(const char * s, uint8_t *buffer, >> + unsigned int size) >> { >> struct in6_addr addr; >> unsigned int family = 0; >> @@ -116,7 +116,8 @@ static int octets_from_ip(const char * s, unsigned int >> *buffer, >> return n; >> } >> >> -static CMPIArray *octets_to_cmpi(const CMPIBroker *broker, unsigned int >> *bytes, int size) >> +static CMPIArray *octets_to_cmpi(const CMPIBroker *broker, uint8_t *bytes, >> + int size) >> { >> CMPIStatus s = {CMPI_RC_OK, NULL}; >> CMPIArray *array = NULL; >> @@ -216,7 +217,7 @@ static int convert_action(const char *s) >> return action; >> } >> >> -static unsigned long convert_protocol_id(const char *s) >> +static uint16_t convert_protocol_id(const char *s) >> { >> enum {NONE = 0, IPV4 = 2048, ARP = 2054, RARP = 32821, IPV6 = >> 34525} id = NONE; >> >> @@ -239,7 +240,7 @@ static void convert_mac_rule_to_instance( >> CMPIInstance *inst, >> const CMPIBroker *broker) >> { >> - unsigned int bytes[48]; >> + uint8_t bytes[48]; >> unsigned int size = 0; >> CMPIArray *array = NULL; >> >> @@ -280,7 +281,7 @@ static void convert_mac_rule_to_instance( >> (CMPIValue *)&array, CMPI_uint8A); >> >> if (rule->var.mac.protocol_id != NULL) { >> - unsigned long n = >> convert_protocol_id(rule->var.mac.protocol_id); >> + uint16_t n = convert_protocol_id(rule->var.mac.protocol_id); >> >> /* Unknown protocolid string. Try converting from >> hexadecimal value */ >> if (n == 0) >> @@ -366,18 +367,19 @@ static void convert_ip_rule_to_instance( >> CMPIInstance *inst, >> const CMPIBroker *broker) >> { >> - unsigned int bytes[48]; >> + uint8_t bytes[48]; >> unsigned int size = 0; >> - unsigned int n = 0; >> + uint8_t ipver_num = 0; >> + uint16_t port_num = 0; >> CMPIArray *array = NULL; >> struct rule_data_t rule_data; >> >> if (strstr(rule->protocol_id, "v6")) >> - n = 6; >> + ipver_num = 6; >> else >> - n = 4; >> + ipver_num = 4; >> >> - CMSetProperty(inst, "HdrIPVersion",(CMPIValue *)&n, CMPI_uint8); >> + CMSetProperty(inst, "HdrIPVersion",(CMPIValue *)&ipver_num, >> CMPI_uint8); >> >> fill_rule_data(rule, &rule_data); >> >> @@ -480,27 +482,27 @@ static void convert_ip_rule_to_instance( >> } >> >> if (rule_data.srcportstart) { >> - n = atoi(rule_data.srcportstart); >> + port_num = atoi(rule_data.srcportstart); > > Hmm.. technically atoi() returns an 'int'... In libvirt code this would > be a call to a function which does a strtoul() and makes sure there's no > overflow, etc. However, since this code is full of other situations > such as this, I'm "OK" with the way it's done... too true ... we have tried to restrict the patch to the big/little- endian fix and not to mix in cleanups. > > >> CMSetProperty(inst, "HdrSrcPortStart", >> - (CMPIValue *)&n, CMPI_uint16); >> + (CMPIValue *)&port_num, CMPI_uint16); >> } >> >> if (rule_data.srcportend) { >> - n = atoi(rule_data.srcportend); >> + port_num = atoi(rule_data.srcportend); >> CMSetProperty(inst, "HdrSrcPortEnd", >> - (CMPIValue *)&n, CMPI_uint16); >> + (CMPIValue *)&port_num, CMPI_uint16); >> } >> >> if (rule_data.dstportstart) { >> - n = atoi(rule_data.dstportstart); >> + port_num = atoi(rule_data.dstportstart); >> CMSetProperty(inst, "HdrDestPortStart", >> - (CMPIValue *)&n, CMPI_uint16); >> + (CMPIValue *)&port_num, CMPI_uint16); >> } >> >> if (rule_data.dstportend) { >> - n = atoi(rule_data.dstportend); >> + port_num = atoi(rule_data.dstportend); >> CMSetProperty(inst, "HdrDestPortEnd", >> - (CMPIValue *)&n, CMPI_uint16); >> + (CMPIValue *)&port_num, CMPI_uint16); >> } >> } >> >> @@ -515,7 +517,7 @@ static CMPIInstance *convert_rule_to_instance( >> const char *sys_name = NULL; >> const char *sys_ccname = NULL; >> const char *basename = NULL; >> - int action, direction, priority = 0; >> + uint16_t action, direction, priority = 0; > > This is declared an uint16_t here; however, later on it's stored as a > signed int16: > > priority = convert_priority(rule->priority); > CMSetProperty(inst, "Priority", (CMPIValue *)&priority, > CMPI_sint16); hm ... collateral damage. The "Priority" property is indeed a sint16 (schema/FilterEntry.mof), "Direction" and "Action" are uint16's. > > > Also in convert_filter_to_instance(), there's some similar oddities: > > ... > int direction = 0, priority; oversight ... should be uint16_t direction = 0; int16_t priority; > ... > CMSetProperty(inst, "Direction", (CMPIValue *)&direction, > CMPI_uint16); > > priority = convert_priority(filter->priority); further, we need to change the return type of all the conver_xxx functions ...
In my opinion this warrants a V2 patch ... do you prefer a resend of entire series or just this single patch? -- 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 _______________________________________________ Libvirt-cim mailing list [email protected] https://www.redhat.com/mailman/listinfo/libvirt-cim
